Как плохо писать на php. Советы профессионалов

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

Как-то я работал на одном потрясающем проекте, некоторые перлы которого приводили меня в ужас и заставляли громко смеяться. Некоторые из них идеально подходят как пример плохого стиля разработки и я с удовольствием поделюсь ими с вами. Ну что, если вы заинтригованы, поехали.

Получив эту работу, я был несказанно рад хотя бы потому, что это была довольно крупная американская компания и наличия знаменитых индийских горе-разработчиков там не наблюдалось. Несколько смутил тот факт, что все проекты пишутся на vanilla php(чистом, без фреймворков), но я подумал, что это не проблема. В приветственном письме было английским по белому написано, что в компании принято использовать PSR’ы (стандарты кодирования для php) и я подумал “ну молодцы”. Получив доступ к репозиторию, я тут же склонил с гитхаба проект и посмотрел как они соблюдают стандарты:

class master_mapping extends basicMapping
class auth
class AdminUser extends auth

То есть спустя ровно минуту я понял что PSR’ами тут даже не пахнет. Здесь нет даже общего стиля. Судя по всему, каждый, кто писал проект, писал его по своему. Не делайте так никогда! Конечно никто вам не запретит писать как вам хочется($varijabl = null;//взято из реального проекта или $peremennaya = 1;), но если вы планируете стать профессионалом, стоит придерживаться единого стиля(хотя бы в рамках одного проекта). Лучше всего, конечно же, почаще заглядывать сюда, в идеале до того момента, пока ваши пальцы автоматически не будут писать код по тем правилам, которые там изложены.

Идем дальше.

class database, в конструкторе которого путем безбожного хардкода устанавливается соединение:

class database
{
public $dbc;
private $host;
private $dbname;
private $username;
private $password;

public function __construct()
{
$this->host = "11.22.33.444";
$this->dbname = "proj_db";
$this->username = "db_uzver";
$this->password = "coolPassword123&";
$this->port = "3306";
$this->dbc = $this->getConnection(false, true);
}

“Они в своем уме?”

…подумал я. Запомните, все подключения к Базе Данных выносить в конфигурационный файл, который не идет под контроль версии, а создается на каждом environment’е отдельно(в идеале нужно настроить CI, но в крайнем случае – создаем файлик конфига ручками на серваке. Взять тот же Laravel, где есть .env.example который можно скопировать с помощью несложной команды cp .env.example .env. В результате имеем файл .env, внутри которого указываем реальные настройки подключения)

Дальше начинаются жалобы 🙂

Немного спешки:

файл getSomeData.php со следующим содержанием:


interface setSomeData
{

Ребята, не реализуйте этот интерфейс в своих классах: проект не запустится. По крайней мере, переименуйте файл(или интерфейс). Как такое попало в репозиторий, особенно если учесть, что в компании присутствует политика пул реквестов и кодревью(и это очень правильно!) оставалось для меня загадкой… Но не долго: оказалась, что по настоящему ревьювили только мои пул реквесты, в то время как штатные сотрудники свои пулы мержили самостоятельно. Вот такая лабуда в итоге в коде и проростала.

Нас(разработчиков по контракту) постоянно ругали за то, что мы приводили их sql запросы к нормальному виду.

то есть им больше нравилось такое:

$sql = "SELECT field1, field2, field2, field3, field4, field5 FROM ...

обычно экрана не хватало, чтобы все это прочесть и приходилось долго и мучительно скроллить вправо или просить вот такие мониторы:

Когда мы форматировали это безобразие:

$sql = "SELECT
	     field1,
             field2,
             field2,
             field3,
             field4,
             field5
         FROM
             table_name
         WHERE
             field6 = 'something';

они находили баги в своих же запросах 🙂 (да неужели??) и просили пофиксить

однажды мой пул не заапрувили вот из-за чего:

…LEFT JOIN users_new u …

comment:

change this to users_new AS u it makes it easier to read

Really??

После этого кстати я имел разговор с техлидом, который пообещал организовать (ВНИМАНИЕ) style guides 🙂 для нас, чтоб такого больше не было. Как вы могли догадаться никаких стайл гайдов не было.

Как вам, кстати, название таблицы пользователей? Знаете почему так? Потому что таблица users по каким-то причинам устарела… (???).

Кстати проблема базы данных на этом проекте была достойна отдельной статьи, но вкратце: есть единственная база данных(продакшн), которую все используют в том числе и для тестирования локально. Никаких миграций. В крайнем случае создавалась новая таблица с постфиксом _copy в конце т.е. что то вроде table_name_copy, которую можно использовать как угодно для разработки и тестирования. А теперь представьте сколько таких таблиц наплодилось, сколько весила БД и как это все тормозило. И опять таки Laravel прекрасен в этом. Отличные миграции, все удобно и понятно. Да, я большой фанат Лары(не Крофт).

Что ж дальше пойдем по извращениям:

$_REQUEST['params'] = json_decode($_REQUEST['params']);
$_REQUEST['networks'] = $_REQUEST['params']->networks;

что здесь происходит не знает никто…

SELECT network_campaign_id AS campaign_network_id

Полезно и читабельно. Видимо это одно из правил тех самых стайл гайдов, которых мы так и не дождались

if($values['link'] === undefined)

Когда кодил на JS и вдруг заставили писать на php. Ну хоть typeof не вставил… Будьте повнимательнее и пишите код в IDE(в идеале PHPStorm), а не в блокноте, чтобы видеть ошибки, которые он вам будет подсвечивать.

if ($userId !== null) {
$this->setUserId($userId);
} else {
$this->setUserId('NULL');
}

нам нужно больше if-else, да и $userId со значением null нам не нравится, а поэтому мы заменим его на ‘NULL’, который почему то в кавычках, видимо потому что undefined из предыдущего примера – без…
Вот это действительно не читабельно. Да и проверка эта ничего не даст. Проще было написать:

$this->setUserId($userId);

так как эта “недовалидация” все равно бесполезна.

  
       /**
	*
	* some method blah blah
	*
	* @param $data
	* @return string
	*/

	}

	public function someFunction() {

так спешил, что пхп доку к следующему методу начал писать в предыдущем…

if($_SESSION['id'] == 26){
$groupIds = "1','2','3','4','5','6','7','8','9','27','30','31','131";

Когда любишь хардкод, но ненавидишь массивы и здравый смысл

$data = [];
while ($row = $query->fetch_assoc()) {
if ($csv === true) {
$row = $data[$row['network_item_id']];
$row['ad_text'] = null;

что тут чему присваивается?

Проверяйте очередность условий. Вот такое вот, например, отработает, но выдаст Notice, который меня горе кодеры попросили пофиксить:

if ((!isset($data['is_active']) || $data['is_active'] == '') && $data['is_active'] != false)

А все потому что если $data[‘is_active’] == null то левое условие отрабатывает, но так как дальше стоит логическое ‘и’ должно отработать и выражение справа, в результате которого мы получим Notice: Undefined index: is_active

по хорошему все это фиксится просто if (empty(data[‘is_active’])), однако как оказалось data[‘is_active’] можеть быть равно 0, поэтому пришлось && заменить на || в исходном выражении:

if (!isset($data['is_active']) || $data['is_active'] == '' || $data['is_active'] == false)

Вот такие перлы бывают на реальных проектах, которые разрабатывают ‘вроде бы профессиональные’ программисты. Наверное именно из-за таких “профессионалов” пхп-разработчиков и считают априори плохими. Уверен, у вас тоже есть куча таких примеров. Делитесь в комментариях. Спасибо за внимания и до новых встреч.

P.S. Да уж, не статья у меня получилась, а скорее крик души, но как бы там ни было, надеюсь кто-то почерпнет из нее что-то важное для себя, кто-то посочувствует :), а кто-то просто посмеется. Желаю всем интересных проектов, адекватных команд и главное, чтобы PSR на ваших проектах были PHP Standards Recommendations, а не П****ским Стилем Разработки.