peer code review and ascii art

[juan_gandhi]

While at Google, I believed in two things: peer code review and unacceptability of ascii art in the code.

Peer code review means you are supposed to show your stuff to somebody in your team before submitting it. So, when you want to submit the stuff in the end of the day (yes, all unittests pass), you are supposed to hold on, wait for half a day (the next day), not touching the code, get into a discussion, and then, eventually get through with it. Which means the code should be pretty solid. No way you would go ahead submitting something working but halfway through, or something that is in the process of being formed, in transition, work in progress.

That's bad. I found myself spending several days forming my ideas regarding how I can possibly specify binary output format in JSON, so that the format can be sent From Above to the client when requirements change (say, a new format is eventually implemented by busy/lazy server guys). If I had to submit a fully-implemented, fully thought-out, neat-looking code, it would take a couple of weeks. Thinking, prototyping, discussing, including three days of explaining how the stuff works and what JSON is, and why it is okay to use strings as keys, etc.

That's about pair programming too. Imagine pair theorem proving. Two guys are placed together by a senior mathematician, say, Grisha and Misha, and told to prove a theorem by the end of the week.

What I want to say: get the fuck off programmers' backs. If they want to work together, let them work together. If they want to work alone, let them work alone. If they want to be agile, let them be agile; if they want to be locally-convex, let them be locally-convex. On the other hand, if you, the manager, know better how to write code, how come you cannot produce in a week anything comparable to what a junior programmer can easily concoct in 30 minutes? Just kidding.

Now ascii art. Programming is art. Ascii art is a part of programming art. If a programmer thinks that his or her code reads better formatted the way the programmer formatted it, challenge this, the fact that it is, not the fact that it does not follow Coding Style Guide Laws.

Comments

[dkfl.livejournal.com]

что-то не понял я аргумент про аски-арт.

[ivan-gandhi.livejournal.com]

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

[yatur.livejournal.com]

По поводу code review - я на практике ни разу не видел эффективного code review. Они либо формальны, либо превращаются в борьбу эго - "а я считаю, что в этой строке слишком много скобочек, мне так непонятно". Особенно это заметно, если проверяющий не понимает, о чем идет речь (а он как правило не понимает, иногда потому, что дурак, иногда потому, что своей работы полно). Ему ничего не остается, как придираться к мелочам.

По поводу coding conventions - это болезнь. Это то, что легче всего формализовать, поэтому большинство организаций, когда перед ними встает вопрос "как лучше программировать" начинают создавать 50-страничные правила именования классов. Объяснения, что это waste of time, обычно не помогают.

Что же касается get off programmer's backs, как Вы думаете, что сделает большинство программистов, если дать им полную свободу? Напишут ли они хоть что-нибудь в обозримые сроки? Я говорю именно про большинство, программистов от Б-га я прошу в расчет не брать - их ничтожные доли процента.

[yatur.livejournal.com]

Желательно, чтобы соблюдалось правило наименьшего сюрприза. Конструкции типа while (x-->1) выглядят красиво, но вводят неподготовленного читателя в ступор там, где не надо. С другой стороны, если в Вашей организации while (x-->1) является хорошо понятной всем идиомой, то это не страшно.

[taganay.livejournal.com]

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

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

[taganay.livejournal.com]

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

[ivan-gandhi.livejournal.com]

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

[6zow.livejournal.com]

Получалось?

[ivan-gandhi.livejournal.com]

В течение примерно 15 лет. Но я был довольно циничен, и выпихивал всякого, кому это было неинтересно.

[aivanov.livejournal.com]

Что там гугл! Вот Дитрих в 2000, после того как прочитал Кента Бека, принял решение отобрать у программистов половину компов и половину столов, чтобы сделать пейр-программинг неизбежным -). Ценой очередного почти-увольнения мне удалось отбиться до состояния "пилотной пары". Потом бедняги должны были всем рассказать как им хорошо. Один на соответствующую презентацию вообще не пришел.
Второй рассказывал так: "Когда мы только начали заниматься ЭТИМ, нам очень не нравилось. Но потом мы привыкли, и сейчас даже не можем себе представить, как можно программировать без ЭТОГО".
Ну, как обычно бывает, после того как запахло жареным в смысле релиза следующей версии к JavaOne экспирименты закончились.

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

[6zow.livejournal.com]

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

[ivan-gandhi.livejournal.com]

Да, парное программирование похоже на секс. Но не заниматься же сексом 8+ часов каждый день.

А Дитрих, видимо, просто решил сэкономить на оборудовании - в голове немецкие тараканы.

[vit-r.livejournal.com]

three days of explaining how the stuff works and what JSON is

Пардон, но программы пишутся не только для себя

[vit-r.livejournal.com]

чтоб его структура была понятна

Нет субъекта. Понятна кому? ;-)

[vit-r.livejournal.com]

При введении любой системы первыми уходят лучшие.

[ivan-gandhi.livejournal.com]

О, действительно.

[ivan-gandhi.livejournal.com]

Да дело не в этом Результат-то потом вполне читабелен, и объяснять ничего не надо, там кода в три раза меньше, чем в оригинале. Дело в том, что при ревью нужно разъяснять каждую стадию художественного процесса.

[vit-r.livejournal.com]

и объяснять ничего не надо

%-)
Сказки какие-то.
И про ревью тоже очень-очень странно. Впрочем, если это мини-пузомерка, тогда без вопросов.

[ivan-gandhi.livejournal.com]

Я вполне серьёзно. Сопоставить два явления:
- сабмиты нужно делать часто, всяко не реже, чем раз в день;
- большой рефакторинг требует времени и определённой свободы исследования

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

Это я поглядел на вещи с обеих сторон. И всем советую. Хотя, возможно, кто-то находится в процессе переосознания, и спешить, требуя от него полного понимания идеи, которая, может быть, и выражена невнятно (т.к. тоже находится в процессе формирования) было бы непрвильно.

Так понятней?

[vit-r.livejournal.com]

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

А внятное выражение мыслей - это и есть задача нашей профессии.

[ivan-gandhi.livejournal.com]

Широким читательским массам.

Вот это вот читабельно?

       public final static Schema HEADER_SCHEMA = 
            new JSONSchema.Array()
                .add(new JSONSchema.ByteConstant(PACKET_TYPE))
                .add(new JSONSchema.ByteConstant(PACKET_VERSION))
                .add(new JSONSchema(Descriptors.App.TYPEID, App.APPNAME, App.VERSION))
                .add(new JSONSchema(Descriptors.Device.TYPEID,
                        new String[] {
                            Device.CARRIER,
                            Device.PLATFORM,
                            Device.NAME}))
                .add(new JSONSchema.ByteConstant(0));

[ivan-gandhi.livejournal.com]

Первое легко покрывается наличием соответствующих юниттестов.

[jakobz.livejournal.com]

Наверное все книжки по организации программистов, нужно начинать и заканчивать этой фразой. А посередине можно ничего и не писать.

Такого менеджера я бы глубоко уважал даже если бы он меня уволил.

[vit-r.livejournal.com]

Юниттест доказывает, что кусок кода работает так, как программист думает, он должен работать. А не так, как должен работать. :-)

[vit-r.livejournal.com]

[ivan-gandhi.livejournal.com]

Эх... ну да, ну да.

[vit-r.livejournal.com]

На самом деле проблема только в friendly/unfriendly testing. Остальное - только технологии.