Нужно ли проверять все аргументы метода/функции?

Question

[Андрей К]

Пишу вспомогательный класс со статическим методом. Метод выбрасывает специфические исключения, если в процессе обработки данных что-то пошло не так. В качестве аргументов принимает специфичные для себя и "системные" (объект Context в Android).

Вопрос: нужно ли проверять все аргументы до одного или только специфичные для этого метода?

У меня две мысли на этот счёт:

1. метод должен быть устойчив к неправильным аргументам, поэтому нужно проверять все;
   
2. задача метода - проверить специфичный для себя аргумент и сообщить пользователю, что с ним что-то не так, а проблемы с "системными" аргументами - забота системы. Исключение всё равно будет выброшено, так зачем проверять их вручную и выбрасывать те же исключения?
   

Косвенным подтверждением того, что не нужно проверять "системные" аргументы, является то, что код с их проверкой становится распухшим и некрасивым...

Пример 1. Красиво, но неустойчиво к неверным аргументам:

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);
        }
	}

}

Пример 2: Учтойчиво, но декларируется много исключений

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);
        }
	}

}

Пример 3: Устойчиво, но исключения-обёртки

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);
        }
	}

}

Comments

[Dmitry Roo]

Что значит "не правильные аргументы"? Не того типа? null?

[Dmtm]

пускай валидацию аргументов делает отдельный валидатор и сводит их все к единственному IllegalArgumentException, все что в методе возникнет во время работы - просто передается дальше,
туда где известно как обрабатывать исключения
и не надо плодить свои классы исключений
PS: ImageLoader_Exception - так тоже не надо

Answer 1

[⚡ Kotobotov ⚡]

мой взгляд -> проверять входящие аргументы желательно вообще на ВСЕХ функциях (особенно на критически важных), другое дело что я бы не бросал исключения (мысль такая, что если ты что-то обработал, то это уже не исключительная ситуация), я бы просто их логировал - и исправлял ошибки на возможно корректные или значения по умолчанию (заглушку).

Answer 2

[Денис Загаевский]

Проверки на null заменить аннотациями @NonNull/@Nullable + опционально requireNonNull.
Context == null это не проблема системы, это проблема пользователя, который туда засунул null. Определись, какие аргументы ты проверяешь на null, а какие - нет.
При создании своих исключений добавляй полезные данные. "Неверный формат URL" - это не полезные данные.
В каком случае будут поврежденные данные приняты, и как ты это проверяешь? Имхо, лишнее.
Проверяемые исключения не нужны(но это не общепринятый факт), и уж точно не надо указывать в списке throws IllegalArgumentException.
При создании своих исключений наследоваться от RuntimeException, а не Exception.
Определись, CamelCase ты используешь, или snake_case. Не надо их смешивать.
И, в конце концов, вообще выкинь эту свою "утилиту", и используй Glide. Пользы точно будет больше.

Answer 3

[Андрей К]

Пораскинул мозгами и пришёл к следующему выводу.

Мой метод принимает данные:
1) пользовательские (URL картинки), которые меняются в процессе эксплуатации программы и которые невозможно предугадать;
2) ссылки на системные компоненты (Context), которые предоставляются на этапе разработки.

Пользовательские данные непредсказуемы на этапе разработки, поэтому их нужно тщательно проверять, сообщать пользователю об ошибках (и писать в лог для разработчика).

Системные компоненты - предсказуемая вещь, они контролируются на этапе разработки. Если программист написал так, что в качестве аргумента Context встало что-то другое, то это грубая ошибка, должно быть выброшено исключение, программа должна упасть. Исключение выбросит сама система, делать дополнительные проверки - лишняя работа.
Для удобства можно всё-таки их проверить и выбросить IllegalArgumentException с осмысленным сообщением, чтобы потом не вышло NullPointerException, с которым придётся разбираться дольше. Но обрабатывать и "поглощать" эти ошибки не нужно, так как это скроет от разработчика ошибки в коде.

Класс, таким образом, будет выглядеть так

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);


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

Comments

[Денис Загаевский]

Код, таким образом, вообще неграмотный. Проверять переменную на её же тип? Парсить стрингу, чтобы проверять на ошибки, когда есть класс Uri? Заставлять пользователя ловить исключение, когда вызывающий код дает что-то не то?
Код плохой.

[Андрей К]

Спасибо за комментарий. С проверкой аргумента на его собственный тип я действительно тупанул. С парсингом URL - неудачный вымышленный пример. В реальности мой метод определяет формат изображения и выдаёт исключение, если этот формат программой не поддерживается.
Проверять аргументы на null, получается, тоже не обязательно, аннотации @NonNull подскажут IDE подсветить опасные места (но не помешают программисту их игнорировать).