пятница, 8 октября 2010 г.

Coding style battle

Стандарт кодирования у нас в команде отсутствует.

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

Да, я могу согласиться что использование using namespace в глобальном пространстве имен чем-то чревато. Может вызвать различные неудобства, в плане того, что описываемая переменная начнет конфликтовать с каким-то именем из указанного пространства. Не вижу в этом ничего страшного, не знаю как кто, я достаточно редко на это натыкаюсь.

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

Но мне не жалко. Я могу пойти навстречу, и соглашусь отказаться от использования using namespace в глобальном пространстве. Хотя с постоянным указыванием std::, boost:: и других общеупотребительных пространств я все равно буду бороться с помощью локальных using и определений типов.

Благо на этом сошлись.

Но есть еще одна вещь на которую я пойтить не могу!

Совершенно дурацкий стиль - ставить тип на отдельной строке. Его ноги вероятно растут из GNU codin style, но GNU - это все таки по большей части си.

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

Вообще я не люблю распространение программы в длину, так типичное для GNU стиля. И каждая строка должна быть самодостаточна. Для функции главным является тип, имя и список аргументов. И это все должно быть вместе.

Если поиск по названию функции выводит 100 мест где она используется, может быть это и правильно, но мне кажется - что-то не так с архитектурой... надо лечить.

Почему у меня не возникает проблем с поиском? Наверное не так что-то делаю...

14 коммент.:

reperio комментирует...

ИМХО это безумно неудобно разбивать код на множественные строки без пущей надобности. Я понимаю если разбивать входные аргументы на строки - это удобно, но возращаемый тип это в принципе ненужно.
У нас так же были подобные войны, но вызваны были таким вот кодом:

for(
unsigned long nii = 0;
(clusterCount > nii) && SUCCEEDED(hResult);
++nii)
{
...

или

This(
/** Pointer to a variable that receives the \c HRESULT status of the
function execution. This parameter could be \c NULL.
*/
HRESULT* const phResult)
throw():
pImpl(NULL),
isPlugIn(int()),
m_pOutlinesListIn(NULL)
{

Это конечно хорошо, но вся команда глаза сломала об такие вот конструкции...

Анонимный комментирует...

Это надо, дескать для того, чтобы легко можно было найти реализацию функции, поскольку перед именем функции в определении нет пробельных символов
Обычно, определение отличается от объявления и использования тем, что у него нет точки с запятой в конце, регэкспы в помощь. Ну и вообще, приличная IDE такой поиск делает в два клика, так что, ИМХО, аргументы ваших оппонентов несостоятельны.

Андрей Валяев комментирует...

Много вариантов может быть, всех не предусмотришь... здравый смысл в этом есть, но просто совершенно не дело оптимизировать исходный код под grep.

С другой стороны нормально названная функция в поиске слабо конфликтует со всякими другими переменными. И легко находится.

Случай, когда функция используется многократно (100 и более раз) - крайне мал ИМХО.

Вот мне и кажется достаточно странной приверженность писать так:

int
foo() {
}

Вот скобочку я всегда открываю на новой строке, ибо к прототипу она не относится совершенно.

Евгений Охотников комментирует...

Я тоже долго не понимал зачем тип возвращаемого значения указывать на отдельной строке. И указывал на той же самой. Пока не запарился со случаями, когда имя возвращаемого типа не совсем короткое. Например, можно сравнить:

virtual const some_template_type_name<some_my_type_with_long_name> get_something();

и

virtual const some_template_type_name<some_my_type_with_long_name>
get_something();

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

А потом уже и к коротким именам типов (вроде void и int) привыкаешь.

Андрей Валяев комментирует...

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

К вопросу о неймспейсах - тип для метода какого-то класса входит в облась видимости этого класса вроде бы.

Не, вру, класс надо указывать... Но всеравно это тоже может сократить лишние буквы... :)

class foo {
typedef some_template_type_name vs;
vs bar();
};

foo::vs foo::bar()

Евгений Охотников комментирует...

>длинные, особенно темплейтные типы, всегда загромождают исходник.

Бывают и не темплейтовые. Вот, из недавнего:

virtual allowed_time_searching_result_t
get_allowed_time_starting_from(
const ACE_Time_Value & tv,
const valid_timezone_t local_timezone,
const valid_timezone_t dest_timezone ) const;

>foo::vs foo::bar()

Имхо, это верный путь запутать читателя программы.

afunix комментирует...

А я боролся и буду бороться с using namespace. Написать std:: не сложно, но при этом сразу показывает, что человек пользуется stl. Ну и конфликтов не будет...
А с переносами в описаниях методов: конструкции типа template на отдельной строки, а тип на той же, что и имя метода.

Андрей Валяев комментирует...

2Евгений Охотников:
Конструкция вида allowed_time_searching_result_t не дает никаких подсказок на тему того, где ее искать...

В то время, как Time::allowed_result_t (Ну я к примеру) Дает нам указания, где искать данный тип.

Я не призываю все сокращать до vs... :)

2afunix: То есть вы допускаете использование имен стандартных типов за пределами std::? Вот что способно серьезно запутать, так это использование string в разных неймспейсах.

ССЗБ не тот, кто написал using namespace std, а тот, кто назвал свой тип string. :)

Евгений Охотников комментирует...

>В то время, как Time::allowed_result_t (Ну я к примеру) Дает нам указания, где искать данный тип.

Не-не-не :)))
В большинстве случаев оно приведет нас к:

typedef allowed_time_searching_result_t allowed_result_t;

После чего, как говорят математики, задача приводится к каноническому виду ;)

Андрей Валяев комментирует...

Может быть..

Но может быть и конкретное определение.

С другой стороны, когда ты найдешь allowed_time_searching_result_t там тоже может быть косвенный typedef... :)

Так что отрицание классового размещения типов вовсе не гарантирует удобоваримости...

Андрей Валяев комментирует...

Получается, что запрет using namespace приводит к увеличению количества промежуточных typedef и using.

Хотя, может быть, для промышленного софта лишняя конкретика - это гуд.

Евгений Охотников комментирует...

>Так что отрицание классового размещения типов вовсе не гарантирует удобоваримости...

Так ведь речь не о том :)
Если имена типов возвращаемых значений длинные (а они часто такие и есть), то их запись на отдельной строке повышает читабельность программы. И не важно, как описан сам тип -- через typedef или нет.

Насколько я помню, в исходном посте такой аргумент не упоминался.

Андрей Валяев комментирует...

Удобочитаемость - тонкая материя. :)

Мне обычно удается умещать в строке прототип целиком... иногда только параметры не влезают, это мелочи.

Если имена типов слишком длинные - это тоже повод задуматься. 30 символов - это уже достаточно длинный тип.

Хотя я и не заморачиваюсь сильно на 80 символах в строке, как некоторые... Всеравно с такими длинными типами особо не разгуляешься.

А открывающую скобку тоже нет смысла прятать за прототипом... она там очень трудно обнаруживаема. ИМХО.

Вообще такого рода дискуссии можно продолжать бесконечно. :) int foo() всеравно лучше читается в одной строчке, чем в двух. :) А где эта граница, отделяющая понятный код от непонятного?.. :)

Собственно int foo() доказывает тот факт, что ради какого-то поиска нет смысла писать

int
foo()

Потому что уродство. :)

А на сложный код всегда надо смотреть критически, дабы как можно выразительнее донести его смысл.

Евгений Охотников комментирует...

>Удобочитаемость - тонкая материя. :)

Ага, так и есть. Поэтому если для одного читабельнее все записывать в одну строку, то второму точно так же удобнее разносить по разным строкам. И каждый из них прав :)

>30 символов - это уже достаточно длинный тип.

Как человек, воспитанный на Pascal-е (еще даже не Delphi), не соглашусь.

>Собственно int foo() доказывает тот факт, что ради какого-то поиска нет смысла писать

Вот с этим согласен. К поиску это не имеет отношения.