В связи с тем, что я часто сталкиваюсь с начинающими разработчиками, решил написать несколько общих советов по работе, так как по моим наблюдениям часто все повторяют одни и те же ошибки.
Все, что я перечислю ниже - это примеры из моего опыта работы, то с чем я сам сталкивался и видел. Не хочу никого обидеть, а хочу указать на то, что часто мешает в работе и вам, и тем, с кем вы работаете на проекте.
Ни для кого уже не секрет, что в основном разработчик больше читает код, чем пишет его.
Ваш код будет кто-то читать и не один раз. И необязательно при этом быть разработчиком популярной библиотеки.
Поэтому при написании кода старайтесь уделять больше внимания его оформлению, именованию переменных, методов, классов.
Чем легче читается ваш код вами - тем лучше. В качестве самоконтроля достаточно делать небольшие перерывы, после возвращаться к написанному коду и спрашивать себя, остается ли он все так же понятен вам.
Подробнее об этом читайте в разделе Оформление.
Не стоит недооценивать важность проблемы, связанной с null. Призванный решить проблемы обозначения отсутствия значения null сам стал огромной проблемой.
В некоторых случаях без него в Java действительно никак, но при этом многие используют его как серебряную пулю, возвращая это значение даже тогда, когда это, казалось бы, не логично.
Например, рассмотрим метод, возвращающий книги по заданному автору.
Часто в Java разработчики в случае, когда книг нет, возвращают null.
List<Book> books = getBooksByAuthor(String author);
if (books != null) {
// do some work
}Так вот, в данном случае абсолютно не логично возвращать null. Нет значений - верните пустую коллекцию.
В Java 8+ появился специальный класс java.util.Optional, призванный помочь в обработке null-значений, хотя и это не является панацеей.
Подробнее про это есть еще в Effective Java, или вот тут.
Проблемы с null настолько непредсказуемы, что становится иногда даже смешно.
Более подробно об этом.
Чем больше свойств и методов класса скрыто, тем лучше.
Посмотрите на код, приведенный ниже, и постарайтесь ответить на вопрос, что в нем не так.
class EventPusher {
private Producer<String, byte[]> producer;
private Properties props;
EventPusher(Properties props) {
this.props = props;
}
public Producer<String, byte[]> createNewProducer() {
// some code
producer = new Producer<String, byte[]>(props);
}
public pushEvent(Event event) {
Producer<String, byte[]> producer = createNewProducer();
producer.send(event);
}
}Подобный код открывает путь в ад, и я уже ползу по разбитому стеклу, чтобы покарать вас.
Почему?
Да ведь вы открыли создание нового producer наружу!
Теперь любой может переопределить ваш метод и создавать producer так, как ему хочется, даже несмотря на Properties props, которые вроде как передаются в конструкторе.
Т.е мы создали возможность переопределить создание продюсера, хотя по архитектуре класса его поведение должно быть определено теми настройками, что мы передаем в конструкторе.
При компиляции такого кода никаких ошибок не будет, при запуске скорее всего тоже. Думаете, все будет прекрасно работать? Нет. Мы просто будем передавать один конфиг, а продюсер будет писать по совсем другим настройкам.
Крайне плохое решение, подверженное трудноуловимым и критическим ошибкам - просто потому, что мы открыли наружу метод, который занимается внутренней логикой класса.
Пока без штрафов - впредь, будьте аккуратнее.
Решение в данном случае крайне простое - закрыть создание продюсера, сделав его не public.
Чтобы понять, что закрывать, а что - нет, смотрите на класс, который вы пишите, и задавайте вопрос, является ли этот метод частью открытого интерфейса.
Класс EventPusher должен предоставлять возможность отправки событий, как следует из его названия. Значит pushEvent - это часть открытого интерфейса. Должен ли этот класс уметь создавать вам новый продюсер? Навряд ли, этого никто не ждет от него. Этот метод нужен только для инициализации продюсера, занимающегося отправкой событий. Значит это закрываем от доступа извне.
И помните: чем более открытый интерфейс предоставляет класс - тем больше вероятность, что все сломается.
Закрывайте все, что можно, от доступа извне.
Несмотря на кажущуюся простоту и удобство, строки таят в себе большое количество опасности.
Часто можно встретить что-то подобное:
if (format.equals("XML")) {
// some code
}Здесь таится сразу несколько проблем.
Первая проблема состоит в том, что format вполне может оказаться null, так что при такой записи перед сравнением строк вам надо еще проверить на null сам format.
Поэтому гораздо практичнее пример выше переписать в виде:
if ("XML".equals(format)) {
// some code
}Но помимо этого возможна еще одна, более трудноуловимая проблема. Ведь в format, значение аргумента может быть:
- Значение "xml", но в нижнем регистре.
- Значение " xml " - с пробелами в начале или в конце.
И хоть оба варианта также по логике будут означать, что работа ведется с форматом xml, но код работать будет по-другому и в if блок выполнение не зайдет.
Для решения этих проблем в Java у String есть методы equalsIgnoreCase, trim, toLowerCase, toUpperCase.
Работая со строками, помните о регистре, о том, что пробельные и непечатные символы также влияют на результат сравнения!
В приведенном же выше примере правильнее, как мне кажется, вообще использовать
enum.
А еще, работая со строками, помните о кодировках.
Еще одна важная проблема, которую я вижу, это тотальное пренебрежение или непонимание работы с исключениями.
Исключения - это опасная штука, которая ломает поток выполнения вашей программы, может влиять на производительность, приводить к ошибкам в работе. Но при этом огромное количество кода и библиотек построено на их использовании, а иногда они и вовсе очень удобны в использовании.
И некоторые просто не понимают как с ними работать, что выражается или в постоянном неконтролируемом генерировании исключений, или в не нужном перехвате ошибок с помощью catch.
В генерировании исключений надо помнить, что исключение ломает поток выполнения программы, и везде, где возможно, лучше обойтись без них.
Но если вам необходимо прервать поток выполнения, то постарайтесь подобрать наиболее близкое исключение из существующих или создайте свое. Помните, что класс исключения описывает ошибочную ситуацию, в которой оно возникло. И худшее, что можно сделать, это сгенерировать исключение, никак не подходящее под ситуацию.
Например, при отсутствии файла по заданному пути кидать java.util.NoSuchElementException. Потому что путь до файла неверный, ошибся кто-то при вводе, но исключение-то говорит о том, что:
/**
* Thrown by various accessor methods to indicate that the element being requested
* does not exist.
*
* @author unascribed
* @see java.util.Enumeration#nextElement()
* @see java.util.Iterator#next()
* @since JDK1.0
*/И это худшее, что можно сделать.
Это был реальный пример из жизни.
Также не очень хорошей практикой является кидать или оборачивать в исключения более широкого типа.
Например:
if (password.isEmpty()) {
throw new RuntimeException("You entered empty password!");
}Проблема тут в том, что выбрасывается наиболее общее исключение - RuntimeException.
Почему это проблема:
- Можно сузить тип исключения до
IllegalArgumentException, ведь проблема в том, что пользователь не ввел пароль. - На более широкое исключение сложнее реагировать, так как помимо нашего "You entered empty password!" мы можем перехватить еще какие-то возникшие
RuntimeException, которые не ждем.
Старайтесь использовать наиболее подходящие и узкие исключения.
При обработке исключений основное правило, которое надо запомнить, это: на исключения надо либо реагировать, либо делегировать, но никогда не игнорировать.
Если вы перехватили ошибку, то значит вы на нее реагируете. Не знаете как реагировать? Делегируйте.
В качестве примера разберем чтение из файла:
public List<String> readAll(String path) throws IOException {
List<String> lines = new ArrayList<>();
try (FileReader fr = new FileReader(path);
BufferedReader br = new BufferedReader(fr)) {
String line;
while ((line = br.readLine()) != null) {
lines.add(line);
}
}
return lines;
}Здесь мы делегировали ошибку, делегировали тому, кто будет использовать наш код. И там точно так же будут думать, что делать с этим исключением: делегировать или реагировать.
Главное - не забывайте закрывать ресурсы.
Используйте
try-with-resources, это позволит вам избежать многих проблем.
Почему мы не перехватили ее в try/catch? Потому что в данном случае непонятно, как реагировать. Вс>, что можно сделать: залогировать ошибку и прокинуть ее дальше.
Точно так же рассуждайте и в остальных случаях. Если сгенерировалось исключение - прокидывайте его до того момента, где вы точно знаете, что с ним делать.
К сожалению, рано или поздно вы им станете, поэтому старайтесь не доверять, но проверять.
Покрывайте свой код проверками, ведь даже null безопасен, если хорошо контролируется.
Старайтесь проверять то, что вам приходит в методы, валидируйте данные, ограничивайте области видимости и использования переменных до максимально узких.
Опять же, в Java 8+ появились классы типа java.util.Objects, которые содержат методы-валидаторы на null, как например requireNonNull.
Во вспомогательных библиотеках, типа Guava, есть классы-валидаторы, например, Verify или Preconditions.
Задумывайтесь о том, чтобы перенести некоторые проверки прямо в set-ры объектов, чтобы не допустить ситуаций, когда полям объекта можно присвоить невалидные значения. Думайте о том, что вы отдаете пользователю, что вы получаете извне, валидируйте все, что только можно, и никому не доверяйте.
Многие начинающие разработчики очень сильно переживают и боятся выглядеть глупыми, отсюда долго сидят на одном проблемном месте и боятся задавать вопросы.
Помните, что задавать вопросы - это более чем нормально. Никто не будет считать вас глупым. Как говорится, не бывает глупых вопросов - бывают глупые ответы.
Вы - часть команды и влияете на продукт. Поэтому, если вы что-то не понимаете и не спрашиваете, то оказываете негативное влияние на процесс, ведь незнанием вы можете легко навредить. А вот спросив и устранив непонимание - вы только помогаете проекту и даже человеку, у которого вы спрашиваете совет, так как он сам, отвечая на ваши вопросы, лучше разберется в проблеме.
Уча других учишься сам.
Однако, это не говорит о том, что по каждой проблеме и чиху вы должны идти и сразу спрашивать совет. Сначала надо обязательно попытаться разобраться в проблеме самому. Сделать несколько подходов, если не получается, перерывов.
Общего универсального рецепта - сколько времени сидеть над проблемой прежде, чем обратиться за помощью, нет, так как все довольно индивидуально. Если проблема небольшая, то сидеть над ней в одиночку более 30-60 минут, мне кажется, смысла нет.
Вполне возможно, что вы уперлись в то, что быстро решаемо и поправимо более опытным членом команд. Потратив на это еще большее количество времени, вы затормозите выполнение своей задачи, а это никому не надо. Гораздо лучше обратиться за советом к более опытному коллеге, решить эту замолвку и продолжить выполнение задачи.
Над более серьезными задачами можно посидеть и подумать, сделать перерыв, но если в течении пары часов ничего не получается - стоит так же задуматься о подмоге.
Также помните, что прежде, чем обратиться за помощью, надо четко и конкретно сформулировать суть проблемы и вопрос.
Но не бойтесь спрашивать! Это абсолютно нормальная практика. Никто не сомневается в вас, в ваших интеллектуальных способностях и в вашей профпригодности.
Помните, что все когда-то были, есть и будут в таком положении непонимающих.
Еще одним важным моментом является то, что если задача выглядит непонятно или страшно, то помните - главное начать. При этом не стремитесь сделать сразу идеально. Доведите до рабочего варианта, а уже после улучшайте существующее решение.
Вообще, постарайтесь привыкнуть к мысли, что идеального кода не бывает.
На мой взгляд, code review - практика крайне полезная и эффективная. И если в вашей команде этой практики нет, то, возможно, стоит задуматься о ее использовании и предложить руководству эту идею.
Помните, что люди, делающие вам code review, это тоже люди и они могут что-то не
заметить/пропустить, ведь им надо вникнуть не только в ваш код, но и в контекст задачи, при этом потратив на это меньше времени, чем вы.
Держите всегда в голове главное - ЭТИ ЛЮДИ ЧИТАЮТ ВАШ КОД, что автоматически отсылает вас к рекомендации по оформлению.
При этом меня часто удивляет враждебная реакция на то, что вам указывают на ошибку или какую-то неточность.
Думаю, не стоит говорить, что это - недопустимо.
Никто не хочет вас оскорбить или обидеть тем, что просит переименовать метод или удалить вообще этот класс.
Помните, что и вашему ревьюеру тоже пользоваться вашим кодом, а значит, раз вы в команде, вы не можете игнорировать его просьбы. Либо исправляйте, либо приводите свои аргументы. Но не игнорируйте.
Иногда при code review можно услышать, что-то типа "это вообще не мой код, я его скопировал из такого-то модуля/такой-то части проекта и ее писал Джошуа Блох и Мартин Одерски".
Помните - даже очень хорошие программисты могут ошибаться и иногда писать не очень правильно, так как они тоже люди. Но если вы копируете его код в проект - уже вы за него в ответе и сказать, что это Одерски так написал, нельзя.
Либо отвечайте за копипасту, либо не копируйте.
Не стоит надеяться на code review, но относиться к нему хорошо - необходимо.
Важным правилом, которое надо стараться соблюдать, является то, что в коммите, относящемся к решению определенной задачи, не должно быть никаких изменений кода, не связанных с ней.
Да, очень хочется по ходу еще сделать какой-то небольшой рефакторинг близлежащего кода, но в рамках коммита по задаче старайтесь сдерживать себя.
Представим ситуацию, когда вам надо обновить версию библиотеки в проекте. По ходу выполнения задачи вы делаете еще небольшой рефакторинг. Но позже оказывается, что обновление библиотеки было зря, например, новая версия оказалась сырой, и эти изменения с обновлением надо откатить. И в этом случае мы теряем и код-рефакторинг, ведь у нас все в одном коммите! И по сути работа по рефактроингу проделана зря.
Лучше разбейте на два коммита такие изменения. В таком случае коммит с рефакторингом мы не потеряем, при необходимости его можно даже cherry pick-нуть в другую ветку.
Следование этому правилу позволит легко работать с кодом, откатывать в случае проблем изменения, и все будут вас любить.
Помните также, что сообщения к коммитам должны быть человекочитаемыми и описывать то, что сделано в коммите.
При выполнении задачи задумывайтесь о том, с какими данными вы работаете, какой реакции от вашего кода ждут как при удачных, так и при неудачных раскладах.
Например, вам передается список путей до файлов со строками, ваша задача получить один файл с общим содержимым.
В идеале все просто: склеили содержимое всех файлов и записали в результирующий файл-агрегатор.
На деле же вы должны задать себе и другим (аналитикам, например) огромное количество вопросов.
Что делать, если какой-то путь до файла ошибочен и файла нет? Что делать, если часть файла удалось прочитать, а часть нет? Насколько это большие файлы? Хватит ли вам памяти просто все прочитать и склеить или надо вводить какой-то промежуточный буфер? Что за кодировка у файлов, одинаковая у всех? Какая? В какой кодировке надо получить результат?
Как видите, даже для такой, казалось бы, простой задачи требуется большое количество уточняющих вопросов, каждый из которых влияет на конечный результат и ваш код.
Подобных примеров огромное количество и многие ошибки связаны как раз с тем, что разработчик не думает о том, с чем он работает и какая должна быть конечная цель.
Даже самый простой пример: вас просят передавать в сообщении текущую дату. Первая ваша мысль должна быть: в каком формате ее ждут?
Чем более дотошны вы будете - тем будет лучше для всех.
Думайте о данных, с которыми работаете, и для чего вы это делаете, какой конечный результат от вас ждут.