Настоящий обзор кода @monee

Всем межгалактический здряв! Эта заметка — ответ на "обзор" кода @monee_bot — Телеграм-бота кошелька Эфириума. Пост можно найти по ссылке. Ответ я решил написать из-за очевидной недосказанности и однобокости комментариев автора оригинального ревью.

Начнем критику с традиционного разбора личности и непредвзятости обзорщика. Раз уж взялся за разбор кода стороннего проекта — то должно быть достаточно доказательств собственной компетентности в Интернете. Об авторе мы знаем две вещи: его зовут Дмитрий, а его хендл — @spynode. Гуглим никнейм и... ничего не находим. Единственное релевантное нам упоминание хендла висит на гитхабе; но, заглянув в списки контрибьюторов репозиториев, видим, что хендл этот принадлежит далеко не русскоговорящему человеку. Готовы доверять обзору подобного "призрака"? С автором мы разобрались, теперь пару слов обо мне.

Блокчейном занимаюсь с 2013 года, запустил более 30 проектов с 2012 года, работал в 4 социальных сетях с бешеной нагрузкой на сервера, проектами, вышедшими из под моего пера, пользуются более 15 миллионов человек из более чем 40 стран. Мое крайнее популярное изобретение — @voicybot, у которого уже более 1.5 миллиона пользователей по всему миру. Писал на C, C++, Delphi, JS, TS, Objective-C, Swift, PHP, Java, Kotlin, нескольких вариантах Assembly, R, Python, Ruby и так далее, и тому подобное. Трейсы большинства тезисов обо мне вы можете найти, просто погуглив. И да, мой код лежит в примерах одной из крупнейших криптовалютных бирж. Обо мне закончили, переходим непосредственно к коду. Статья будет критической, придираться буду сильно. Поехали.

PHP

Ну-ну, PHP в 2017 году. В продукте, заявленном на зарубежный рынок, использовать технологию, изжившую себя в начале десятилетия. Используя те же Ruby on Rails, Django или Node.js на TypeScript, кода можно было бы написать на порядок меньше, начиная с новомодных конструкций языков, заканчивая отсутствием $ перед использованием каждой переменной.

Если вы уже кинулись писать гневный комментарий, что PHP — это не зашквар, то, пожалуйста, потратьте 2 минуты и в начале комментария опишите свой опыт разработки, сколько проектов с DAU больше 10 000 пользователей написали (со ссылками на них, конечно же), на скольких языках программирования и как долго писали. Если по своим регалиям вы сочтете себя достаточно аккредитованным специалистом — с удовольствием вступлю с вами в разумный диалог о том, хорош или нет PHP. На комментарии по вопросу PHP, которые не начинаются с описания вашего опыта, отвечать не буду. Как правило, топят за PHP люди, которые либо никогда не запускали хотя бы мало мальски популярных сервисов, либо писали только на PHP всю жизнь. Вы прямо, как программист на 1С, который как-то раз спросил, сколько 1С программисты получают в Германии. Ага.

Стиль и форматирование

Честно скажу, очень неприятно видеть проект, заявленный, как open source, и без предварительной правки стиля и форматирования каким-нибудь beautifier'ом. Добавить его в проект — дело 2-3 минут, настроить — дело 10-15 минут, ценность для читающего код — бесконечна. Выкладывать проект в открытый доступ для, я так понял, доказательства честности разработчиком без стабильного форматирования и стиля — это моветон.

Readme

Выкладывать проект в открытый доступ, не потратив 1-2 часа на написание подробного Readme — это плевок в лицо сообщества. Вы выкладываете код не для себя, а для других людей. "Todo" в Readme недопустимы.

Общий contribution

7 коммитов для такого важного проекта, где люди хранят свои деньги? Коммитов должно быть хотя бы 200-300, судя по количеству кода. Ни одного другого репозитория у разработчика, который выложил бота в открытый доступ? Создается впечатление, что это его первый проект. Я бы деньги не хранил в первом серьезном проекте неизвестного программиста.

Файлы идентификации репозитория

Шаблонные composer.json и package.json без какого-либо указания на название проекта, авторов, репозиторий и на остальную немаловажную для сообщества информацию — это, опять же, неприлично. Это чаще всего происходит именно из-за неопытности разработчика — это же его первый open source проект, что с него взять? Но деньги свои мы там хранить будем, да.

Комментарии

... или их отсутствие. Я не знаю, где оригинальный ревьюер нашел хорошо документированный код — наверное, он не ушел дальше первых двух-трех файлов. Бо́льшая часть кода не документирована. Это еще раз указывает на работу только одного программиста над кодом — и только этому программисту в итоге и будет известно, что, где и как происходит. Хотя бы файл с примерами переменных окружения прокомментировали. Код без комментариев — мертвый код.

Тесты

Любой финансовый продукт — повторю: любой — должен иметь за собой автоматизированные тесты. Иначе ему нельзя доверять. В этом репозитории нет ни единого теста, что еще раз говорит нам об уровне профессионализма разработчика и о том, насколько ненадежной может оказаться платформа. Отсутствие тестов — это глобальный жирнющий минус.

Singleton

Пробегаемся поиском по репозиторию с ключевым словом "Singleton". О, Господи! Да все приложение — Singleton. Прощай, масштабируемость. Прощай, децентрализация серверов. Прощай, принцип отказоустойчивости. Прощай, stateless.

Gitignore

Господи, да вы посмотрите на содержимое вот этой папки! Я в жизни не видел столько .gitignore файлов. Либо разработчик не знал, как работает gitignore — один на весь репозиторий — либо он какой-то извращенец. Фу таким быть.

Модели данных

Судя по тому, что из моделей данных в репозитории только User — разработчик не умеет в хранение и передачу информации. Посоветовал бы ему еще почитать про SOLID, да он сам к этому придет через лет 5-6 разработки, учитывая, как быстро движется PHP в сторону нормального языка программирования.

Но сам факт того, что ни транзакция, ни кошелек не выведены в отдельные классы, даже в виде фасада над уже существующими классами в использованной сторонней библиотеке, огорчает Павла Валерьевича.

Ветки git

... или их отсутствие. Где develop ветка, где релизы, где все то, за что мы любим GitHub? Почему это не используется разработчиком? Где хотя бы какие-то намеки на Continuous Integration? Или код пушится в продакшн без прогона на тестовом сервере? Вы зажали $20 на отдельный сервер для тестов и $10 на CI? Ради чего?

Общая целостность кода

То, что код сейчас ни в каком виде запустить не получится, и то, что большинство файлов проекта пустые или содержат 1-2 функции, а единственное, что более-менее написано — это бот в Телеграме (и то достаточно грязно) — может свидетельствовать о том, что версия, выложенная в открытый доступ, устарела. Ай-я-яй — выкладывать старую версию кода для того, чтобы подтвердить свою прозрачность.

Вывод

Процитирую Евгения: "Обожаю такие штуки, мы сделали кошелек для Эфира в Телеграме, опубликовали код, люди делают на него ревью подробнейшее!"

Процитирую оригинального ревьюера: "Еще раз хочется сказать, что в целом все сделано довольно таки хорошо!"

Как же все казалось радужно и хорошо — пока не пришел нормальный программист с достаточным опытом разработки масштабных проектов и не сделал настоящее code review. ИМХО, пока те примитивные вещи, которые я описал в своем недообзоре, не будут поправлены, мелочи, на которые указал оригинальный ревьюер, особого значения не несут.

Честно сказать, я офигеваю. Такое ощущение, что мир сошел с ума. Один недопрограммист написал портянку на PHP, а второй недопрограммист на PHP посмотрел одним глазом, оценил, и сказал: "Вроде, норм". А пипл хавает.

В итоге: ревью оригинальное совсем не подробнейшее, сделанное сквозь пальцы, чтобы не видеть ничего плохого; а, в целом, все сделано довольно-таки хреновастенько.

С вами был Никитка. Подписывайтесь на мой канал @golden_borodutch. Я как-раз пишу очередного бота авто-трейдера для криптовалютных бирж.

У меня на сегодня все.