A
A
Andrey K2020-01-14 09:57:13
Java
Andrey K, 2020-01-14 09:57:13

Do I need to check all method/function arguments?

I am writing a helper class with a static method. The method throws specific exceptions if something went wrong during data processing. It takes as arguments specific to itself and "system" (the Context object in Android).
Question: do I need to check all arguments up to one or only those specific to this method?
I have two thoughts on this:

  1. the method must be resistant to invalid arguments, so everything must be checked;
  2. the purpose of a method is to check a self-specific argument and inform the user that something is wrong with it, and problems with "system" arguments are the system's concern. The exception will still be thrown, so why check them manually and throw the same exceptions?

An indirect confirmation of the fact that it is not necessary to check "system" arguments is that the code with their check becomes swollen and ugly ...
Example 1. Beautiful, but unstable to invalid arguments:

public static class ImageLoader {

  public static void loadImage(String url, Context context) throws ImageLoader_Exception {
    
    if (isEmpty(url))
      throw new WrongURL_Exception("Неверный формат URL");
    ...
    throws CorruptedData_Exception("Полученные данные повреждены");
    ...
  }

  // Собственные классы исключений
  public static class WrongURL_Exception extends ImageLoader_Exception {
    public WrongURL_Exception(String message) {
            super(message);
        }
  }
  public static class CorruptedData_Exception extends ImageLoader_Exception {
    public WrongURL_Exception(String message) {
            super(message);
        }
  }
  public static class ImageLoader_Exception extends Exception {
    public ImageLoaderException(String message) {
            super(message);
        }
  }

}


Example 2: Robust but declares many exceptions

public static class ImageLoader {

  public static void loadImage(String url, Context context) throws 
    ImageLoader_Exception ,
    IllegalArgumentException 
  {	
    if (isEmpty(url))
      throw new WrongURL_Exception("Неверный формат URL");
    
    if (null == context)
      throw new IllegalArgumentException("Context не может быть null");

    ...
    throws CorruptedData_Exception("Полученные данные повреждены");
    ...
  }

  // Собственные классы исключений
  public static class WrongURL_Exception extends ImageLoader_Exception {
    public WrongURL_Exception(String message) {
            super(message);
        }
  }

  public static class CorruptedData_Exception extends ImageLoader_Exception {
    public WrongURL_Exception(String message) {
            super(message);
        }
  }

  public static class ImageLoader_Exception extends Exception {
    public ImageLoaderException(String message) {
            super(message);
        }
  }

}


Example 3: Stable, but wrapper exceptions

public static class ImageLoader {

  public static void loadImage(String url, Context context) throws ImageLoader_Exception {

    if (isEmpty(url))
      throw new WrongURL_Exception("Неверный формат URL");
    
    if (null == context)
      throw new WrongContext_Exception("Context не может быть null");

    ...
    throws CorruptedData_Exception("Полученные данные повреждены");
    ...
  }

  // Собственные классы исключений
  public static class WrongURL_Exception extends ImageLoader_Exception {
    public WrongURL_Exception(String message) {
            super(message);
        }
  }
  public static class WrongContext_Exception extends ImageLoader_Exception {
    public WrongContext_Exception(String message) {
            super(message);
        }
  }
  public static class CorruptedData_Exception extends ImageLoader_Exception {
    public WrongURL_Exception(String message) {
            super(message);
        }
  }
  public static class ImageLoader_Exception extends Exception {
    public ImageLoaderException(String message) {
            super(message);
        }
  }

}

Answer the question

In order to leave comments, you need to log in

3 answer(s)
⚡ Kotobotov ⚡, 2020-01-14
@angrySCV

my opinion -> it is desirable to check incoming arguments on ALL functions (especially on critical ones), another thing is that I would not throw exceptions (the idea is that if you processed something, then this is no longer an exception), I would just logged them - and corrected errors for possibly correct or default values ​​(stub).

D
Denis Zagaevsky, 2020-01-14
@zagayevskiy

Replace null checks with @NonNull/@Nullable annotations + optionally requireNonNull.
Context == null is not a problem of the system, it is a problem of the user who put null there. Decide which arguments you are checking for null and which are not.
When creating your exceptions, add useful data. "Invalid URL format" is not useful data.
In what case will corrupted data be accepted, and how do you check it? IMHO, too much.
Checked exceptions are not needed (but this is not a common fact), and certainly not in the throws list of IllegalArgumentException.
When creating your own exceptions, inherit from RuntimeException, not Exception.
Decide whether you are using CamelCase or snake_case. You don't need to mix them.
And, in the end, throw out this "utility" of yours altogether, and use Glide. There will definitely be more benefits.

A
Andrey K, 2020-01-15
@aakumykov

Poraskinul brains and came to the following conclusion.
My method accepts data:
1) user data (URL images), which change during the operation of the program and which cannot be predicted;
2) links to system components (Context), which are provided at the development stage.
User data is unpredictable during development, so it needs to be carefully checked, errors reported to the user (and logged to the developer).
System components are a predictable thing, they are controlled during development. If the programmer wrote in such a way that something else was used as the Context argument, then this is a gross error, an exception should be thrown, the program should crash. The exception will be thrown by the system itself, doing additional checks is extra work.
For convenience, you can still check them and throw an IllegalArgumentException with a meaningful message, so that later you don’t get a NullPointerException, which will take longer to deal with. But it is not necessary to process and "absorb" these errors, as this will hide errors in the code from the developer.

public class ImageLoader {
  public static void loadImageToView(Context context, String imageURL) 
  // Пугаем исключением, чтобы донести до пользователя ошибку
  throws ImageLoaderException 
  {
    // Выбрасываем IllegalArgumentException, но не предупреждаем о нём: 
    // программа должна упасть, если программист накосячил
    if (! context instanceof Context)
      throw new IllegalArgumentException("Некорректный аргумент Context: "+context);

    // Сообщаем пользователю о пустом URL
    if (!isEmpty(imageURL))
      throw new NoURL_Exception("imageURL не может быть пустым");

    // Сообщаем о некорректном URL
    if (!validFormat(imageURL))
      throw new WrongURL_Exception("Некорректный imageURL: "+imageURL);


    // ...
    // Собственно работа
    // ...
  }
}

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question