Code review или WTFs/Minute

Oleg Bielkin
3 min readAug 1, 2017

--

Попало мне на code-review “творение” стажера. Не просто стажера, а сертифициорванного (Java certified developer и все такое). Суть задачи очень проста: на сервере есть файл неопределенного размера. Его нужно передавать на клиента блоками по N килобайт при этом, не вычитывая весь файл в память — ибо клиентов у сервера много (и их количество может увеличиться), а количество памяти — величина постоянная.

Код написан местами сложно (там и архитектурные особенности, и стажер наш “не пальцем делан” — шаблоны всякие умные, стратегии, декораторы и прочие factory) и читать его любопытно. Но когда понимаешь, как это все в конце концов работает становится смешно.

Что же сделал наш герой!? Он (то ли с перепугу, то ли не нашел как, а спросить побоялся, то ли от непонимания что он на самом деле делает) — вычитывает таки весь файл в память сервера (чего ему строжайше говорили не делать, в byte[] — массив байт), а далее чтобы это все снаружи выглядело “как будто” он ничего криминального не делает он это оборачивает в InputStream (а конкретно в ByteArrayInputStream) — “обертка” Java над файлами, в основном, предназначенная именно для того, чтобы весь файл за один “удар” в память то и не вычитывать.

Дальше — больше. Ему понадобилось узнать размер файла. С этим он поступил просто. Ну, вы поняли — “вылить воду из чайник и свести задачу к классической”. Он же умеет файл в перегонять в массив, а у массива есть рамерность. Т.е. для проверки размерности файла (а чаще всего ему нужно узнать есть ли файл вообще, а даже не его конкретный размер), он вычитывает весь файл в память и проверяет размерность массива (и зачастую после этого “забывая”, что он этот файл когда-либо читал, а при повторной просьбе “дай таки файл” — вычитает его с диска заново).

Т.е. если не “залезть в самое нутро кода”, то с наружи это все выглядит крайне пристойно и ровно так как и просили. Вот API и ничего как-бы криминального, а “под крышечкой” нас ждут чудеса.

Казалось бы типичная ситуация со стажерами. Но нет же. Это Java Certified Developer. Этот человек знает про API Java больше чем я (по крайней мере в каких-то “тонких” аспектах) и уж точно его знания лучше систематизированы чем мои (опять же с предыдущей оговоркой). И как он такое написал (пока не признается :)!? Он же не просто стажер который “hello world!” умеет писать. Он сдал экзамены Oracle, получил сертификат(ы), работал до нас над другими проектами.

Просто в голову не помещается. А у вас какие были смешные случаи с code-review?

P.S. Для тех кто не знает этот бородатый анекдот про чайник:

Задачка.
Дан пустой чайник. Необходимо вскипятить в нем воду.
Решение нормального человека:
Налить воду в чайник, включить плиту, поставить чайник на плиту. Вода закипит.
Решение ИТшника:
Налить воду в чайник, включить плиту, поставить чайник на плиту. Вода закипит.
Изменим условие задачи. Дан чайник с водой. Необходимо вскипятить в нем воду.
Решение нормального человека:
Включить плиту, поставить чайник на плиту. Вода закипит.
Решение ИТшника:
Вылить воду и свести задачу к классической.

--

--