Демонстрировать плохой код клиенту?

129

Клиент попросил меня сделать редизайн своего веб-сайта, приложения ASP.NET Webforms, разработанного другим консультантом. Это казалось относительно простой работой, но, посмотрев на код, стало ясно, что это не так.

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

Моя работа состоит в том, чтобы просто обновить HTML и CSS, но большая часть HTML генерируется в бизнес-логике и будет кошмаром, чтобы разобраться. Моя оценка по редизайну длиннее, чем рассчитывал клиент. Они спрашивают, почему так долго.

Как я могу объяснить своему клиенту, насколько плох этот код? По их мнению, приложение работает отлично, и его редизайн должен быть быстрым. Это мое слово против предыдущего консультанта. Как я могу привести простые конкретные примеры, понятные нетехническому клиенту?

Обновить

Спасибо за все ответы. Демонстрация атаки с использованием SQL-инъекций имеет смысл, и я продемонстрирую ее в тестовой среде. Это только одна часть многих проблем в этом приложении. Я искал способы объяснить, почему другие части (такие как html, генерируемый в слое данных) должны быть заменены лучшими практиками для обновления html и css. Здесь есть много хороших предложений, которые я соберу вместе, когда поговорю со своим клиентом.

jtiger
источник
97
Продемонстрировать атаку SQL-инъекцией?
Остин Хенли,
30
This application was not written well. At all.Они почти никогда не бывают. :)
хайлем
15
Помимо демонстрации проблем, как говорит Остин. не стоит недооценивать силу доски и маркера. Большинство людей хорошо реагируют на плохой дизайн, объясненный, когда он в форме изображения.
Sirex
3
если он не большой - перепишите его, если он большой - не принимайте
ren
4
Клиент говорит редизайн, и они думают HTML / CSS. Я бы использовал термины «отсутствие модульности» и подчеркнул «логический дизайн» против «презентации». Метафоры строительной конструкции полезны. To make a change in the look of the living room, I had to go into the air-conditioning system.В хорошем модульном дизайне таких вещей не бывает.
Fuhrmanator

Ответы:

144

Нетехники не идиоты (по большей части). Они могут понять технический аргумент, если вы держите его на достаточно высоком уровне. Выберите задачу, которая, по вашему мнению, должна быть простой, и объясните, почему это не так.

Я ожидал, что это изменение будет одним словом в одном файле. Наиболее вероятное место, чтобы изменить это, казалось, было здесь, но когда я изменил это там, это работало только в одном месте, и это сломало эти 7 других мест. Когда я исправил один, он сломал еще два места, вызывая эффект домино, поэтому изменение, которое я думал, должно было занять 10 минут, а в итоге заняло 2 часа. Это только один пример. Там есть много более неожиданных двухчасовых заданий.

Карл Билефельдт
источник
10
Учитывая содержание стольких сообщений об ошибках, «оно сломало __ больше мест» действительно кажется лучшим способом описать эффект домино ...
Изката,
4
Хорошо, я бы больше связал время и стоимость. Покажите им, сколько вы ожидали, что изменение будет стоить в сравнении с тем, сколько это стоило. По моему опыту, клиенты редко обращают внимание, если только вы не можете доказать, что они тратят вдвое, втрое или больше, чем в противном случае.
Тим О'Брайен
87

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

Уязвимости в безопасности - другое дело.

Лично я бы сделал оценку, основанную на требуемой работе, используя существующую структуру и стиль, и пояснил бы, что есть существенные проблемы с базой кода. Я бы отдельно поднял вопрос о безопасности: сделайте демонстрацию взлома базы данных, чтобы показать точку зрения во время встречи.

Мне было очень приятно делать это с предыдущим клиентом с системой подарочных карт лояльности, когда я положил 5000 фунтов стерлингов на «мою» карту и попросил его проверить карту на своем кассе.

Michael
источник
38
+1 демонстрация того, насколько плохой может быть атака SQL-инъекцией. Сделай это перед ними. Если возможно, запишите на видео их реакции.
Филипп
40
@Philip: ... демо должно быть предпочтительно в изолированной среде разработки для приложения. Удаление их производственной базы данных доказало бы это, но может привести к потере контракта (и получению судебного иска).
FrustratedWithFormsDesigner
19
@FrustratedWithFormsDesigner, если у них даже есть среда разработки, доступная ...
Хитрый урод
3
@FrustratedWithFormsDesigner: конечно, очистка базы данных не рекомендуется, какой бы простой и драматичной она ни была. Но для них может быть столь же удивительным извлекать личные данные, а затем (осторожно) изменять некоторые суммы (например, остаток на подарочной карте, как это сделал @Michael). Для дополнительных очков, сделайте очевидным, что вам не нужно видеть код; начните с вывода списка таблиц, выберите несколько интересных имен, дамп содержимого. Это не должно занять слишком много, чтобы уточнить, что это так уязвимо.
Хавьер
76

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

Главный красный флаг здесь!

Если клиент попросит вас не вносить никаких изменений, кроме тех, на которые вы согласились (HTML и CSS), я передам этот проект и отзову свою ставку.

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

Вы можете потерять гораздо больше, чем вы можете получить.

Стив
источник
14
+1 за просмотр более широкой картины. Если вы поработаете над этим и скажете, что все готово, вы можете понести некоторую ответственность за ошибки и проблемы с безопасностью, даже если вы их унаследовали. Если бы кто-то манипулировал моими тормозами, а механик починил мой велосипед и просто снял с себя проблему, я мог бы также подать в суд на них ...
sleske
2
+1 это консультант по уроку, который требует слишком много времени, чтобы усвоить его (и, по общему признанию, это сложная концепция, которую необходимо реализовать в условиях жесткой экономики). Ценность вашего экспертизы зависит не только от работы, которую вы выполняете, но и от работы, от которой вы отказываетесь.
Тим О'Брайен
2
+1 Это урок, который я усвоил трудным путем, и из-за этого мой первый бизнес едва не рухнул. Часто в этих случаях стоимость перечисления всех «дефектов» и цитирования по их устранению требует больше усилий, чем клиент готов заплатить.
Catharz,
30

Объясните и, возможно, продемонстрируйте недостаток.
Когда вы говорите против него, все, что вы говорите, может быть просто горячим воздухом, насколько они обеспокоены. Как только вы покажете им, как их приложение может быть использовано с помощью SQL-инъекции, вы внезапно станете человеком, которому доверяют. Вам нужно доверие, чтобы пересмотреть. И этого достаточно для того, чтобы изменить это, чтобы дать вам это.

Будьте милосердны по отношению к своему предшественнику.
Это не значит, притворяться, что ошибок нет, но если вы встретите снисходительность, то потеряете доверие. Не говорите ни слова о программисте, разве что, чтобы дать ему преимущество сомнения. Сосредоточьтесь на коде, а не на кодере. Заставив их почувствовать себя «хорошим парнем», вы получите гораздо больше свободы в переговорах. И «хорошие парни» никогда не говорят подлых вещей. Объясняя клиенту существующие ошибки безопасности (например, уязвимости в SQL-инъекциях), я предпочитаю говорить что-то вроде этого:

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

Вот и мы. Ни слова зла о разработчике; он просто «большинство программистов», что означает, что он в довольно хорошей компании. И теперь вы продемонстрировали, что вы не «большинство программистов», которые дают вам немного больше доверия и, возможно, повод для них платить вам больше денег.

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

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

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

tylerl
источник
8
+1 за то, что никогда не был снисходительным. Просто позвольте фактам говорить самим за себя ...
Слёске
4
+1 за «Будь благотворительным по отношению к своему предшественнику».
msanford
19

Я начал это как комментарий, потому что сначала я думал, что это было в стороне, но, вероятно, на самом деле это не так.

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

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

Конечно, вы можете указать исходный контроль исходного кода, который вы унаследовали, если это когда-нибудь произойдет, но будет гораздо проще указать на этот документ и сказать более профессионально: «Видите? Я же вам говорил».

Этот документ может стать отправной точкой для дальнейших обсуждений, и он может даже использоваться вашим клиентом, чтобы заставить «нужных людей» дать разрешение на внесение некоторых или всех изменений.

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

Вонко вменяемый
источник
Будем надеяться, что они на самом деле используют систему контроля версий.
Бернард
6
Отличный ответ. Но, как тот, кто был в суде по аналогичной ситуации, которая включала полную документацию и подпись клиента, это все равно стоило мне много денег и головной боли.
Стив
5
Хорошая идея в принципе - однако, обратите внимание, что это может быть много работы. Вероятно, это практично только для больших работ, иначе вы потратите 50 часов на документирование проблем для работы, где вы можете только выставить счет 20.
sleske
@sleske: согласился, что это будет много работы, но, надеюсь, также поможет вам, если случится наихудший случай, и если будет нарушение безопасности. По крайней мере, вам нужно что-то, что говорит о том, что вы видите риски безопасности и что вы не хотите нести ответственность за эти ранее существовавшие риски.
Вонко вменяемый
2
@WonkotheSane: правда, но только если вы берете проект . Если проблемы настолько велики, а ваша запланированная работа настолько мала, может быть лучше просто отказаться от проекта. Конечно, вы все равно должны документировать свои проблемы (безопасность и другие), но если вы никогда не работали над проектом, не должно быть никакого риска ответственности. В конечном счете, вам придется оценить, готов ли ваш клиент оплатить стоимость уборки.
слеске
14

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

Используйте примеры из реальной жизни, чтобы проиллюстрировать эти проблемы, такие как поломка машины или стиральная машина, нуждающаяся в ремонте. Чтобы указать, это использовать примеры, с которыми они уже знакомы. Для объяснения внедрения SQL я просто продемонстрировал бы, что это такое и почему это проблема.

В конце вы хотите сообщить, что заботитесь об успехе приложения, над которым вас просят поработать.

Бернард
источник
3
Это не что иное, как сломанный автомобиль, если только автомобиль не был построен из случайных частей механиком-любителем. Это похоже на гараж, построенный некомпетентным подрядчиком, и владелец хочет, чтобы ОП установил автоматический открыватель дверей. ОП обнаруживает, что гараж небезопасен и нуждается в серьезной доработке немедленно.
Кевин Клайн
2
Представьте себе сломанный автомобиль, который использует скотч для скрепления деталей и не дает приборной панели отображать какие-либо предупреждения или предупреждения водителю, в то время как рулевое колесо может упасть в любой момент. Это требует некоторого творчества, но можно использовать различные аналогии, чтобы проиллюстрировать проблему.
Бернард
Или обратите внимание на «нестандартный» кабель ускоряющего шпагата, который можно привязать к консоли для ручного ускорения. Технология «летать по проводам» по дешевке. Может быть применимо практически все, что они сделали на Red Green Show. То, что у них есть, «работает», но это не красиво, и при поверхностной проверке кажется, что оно хрупкое и увеличивает риск любых изменений.
JustinC
1
Если бы я мог добавить дополнительные голоса к этому, я бы просто сказал: «Помните, что клиент обращается к вам за помощью в поддержке их заявки».
Даниэль Холлинрейк
7

Мне нравится использовать аналогии, к которым может относиться клиент. Объем работы, которую я ставлю заранее, чтобы получить работу, будет зависеть от суммы денег, которую клиент намеревался потратить (100 долларов сильно отличаются от 20 000 долларов). Заметьте, я сказал "намерение" Ваша личная оценка стоимости не имеет большого значения, если вы не понимаете, о чем просите.

В вашей ситуации - опять же в зависимости от денег - я мог бы нарисовать прямоугольник с одной линией, выходящей с каждой стороны, и сказать клиенту: «Вот как вы теперь визуализируете программное обеспечение. Данные идут с одного конца, а выходят с другого, все выглядит красиво, чисто и просто ». «Это то, что, как вы думаете, программное обеспечение выглядит изнутри», а затем нарисуйте третью линию, соединяющую две линии внутри поля.

Затем я бы нарисовал еще одну рамку, такую ​​же, как первая, с линиями ввода и вывода снаружи, но на этот раз я бы сказал: «Вот как на самом деле выглядит программное обеспечение внутри коробки прямо сейчас». и затем, чтобы соединить две линии, я нарисую случайную кучу беспорядка спагетти, возможно, с разрывами, соединениями и каракулями.

Наконец, я бы сказал: «Теперь, что вы просите меня сделать, это ...» и нарисуйте простую фигуру внутри первого прямоугольника, возможно, маленького полукруга, касающегося линии, а затем скажу: «Но для этого я Я должен был сделать это ... "и нарисовать торнадо, похожий на спираль, вокруг линии и продолжить ..." чтобы обойти все это ..... "и указать на спагетти в другой коробке.

Я бы подумал, что это сработает примерно через 2 минуты. Если они все равно настаивают на том, чтобы вы это делали, запишите это, как уже упоминалось выше.

Заключенный 13
источник
6

Как я могу объяснить своему клиенту, насколько плох этот код?

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

Это мое слово против предыдущего консультанта, так как я могу привести простые, конкретные примеры, понятные нетехническому клиенту?

Вы правы, это слово против любого визуала, который предыдущий консультант создал в их головах. Я предлагаю сделать именно то, что вы просите, привести простые, конкретные примеры. Поскольку это редизайн, покажите, как фрагмент HTML, определенный в скомпилированном коде, отображается с остальной частью HTML-страницы и как изменение, которое влияет или не влияет, на остальную часть страницы. Возможно, тот же скомпилированный код отображает разметку после применения некоторого «делового» правила. Покажи разницу.

Это сложная и ОЧЕНЬ распространенная проблема. Удачи с этим.

Джои Герра
источник
6

Будь честным и будь прямым.

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

Майкл
источник
3

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

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

«Смотрите здесь эту штуковину, которая анализирует переменные поста и затем отправляет этот компонент в кроличью нору if-elses? Нет только одного из них, есть один из них на каждой странице (или что-то еще), некоторые из них дезинфицировать входные данные, а некоторые нет (или все не делают), и, не читая всего, я не могу знать, какой ".

nwellcome
источник
«Представьте, что их сайт - это физическая машина, похожая на механический печатный станок» - и это печатает деньги! Но, поскольку он сломан, он печатает не столько денег, сколько мог бы ... это должно их зацепить, -)
Mawg
2

Еще один момент, о котором на самом деле не упоминалось, это то, что в этом случае вы можете просто переступить через то, чего действительно хочет от вас ваш клиент. Чрезмерное достижение - это здорово, и оно может доставить вам много удовольствия. Но если клиенту просто все равно, он думает, что текущая производительность является «достаточно хорошей» и просто хочет каких-то незначительных обновлений, может оказаться невозможным убедить его сделать большие инвестиции в вас, чтобы пересмотреть базу кода.

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

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

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

Скелли
источник
0

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

Никогда не забывайте, что, в сущности, вы просите их заплатить вам, чтобы сделать вашу собственную жизнь проще, по их мнению, просто необходимо найти «парня», который может использовать X по цене Y и увеличивая сложность вашего проекта, может просто исключить возможность для тебя. Это трудный путь, когда вы переписываете месяц и встречаетесь с первоначальным разработчиком только для того, чтобы понять, что все приложение было написано в крайне ограниченном окне разработчиком, который полностью понимал все достигнутые компромиссы. Если приложение выглядит ужасно, но внешне хорошо работает, как вы говорите, вполне вероятно, что так оно и будет. Часто технический долг в кодовой базе является продуктом ресурсных ограничений, в которых был разработан код, и если они не создают команду и вместо этого заключают контракты ...

Я просто говорю

khrome
источник
0

Я собираюсь сыграть здесь адвоката дьявола (что-то вроде того, что говорит @khrome: «Вы не платите клиентам, чтобы облегчить свою жизнь»). Я бы даже сказал, что представленный вами случай слишком односторонний, потому что вы описали случай в общем виде. Большинство новых консультантов в новом проекте будут плохо освещать предыдущий проект ... Я не говорю, что это то, что вы делаете здесь, но пока мы не увидим примеры, я не могу просто поверить на слово.

Тем не менее, я собираюсь попытаться решить проблемы с вами по пунктам:

  • SQL-инъекции . Итак, я предполагаю, что программист использовал конкатенации строк вместо параметризованных запросов и / или хранимых процедур. Это очень легко исправить, особенно в ADO.NET ... Я лично упомянул бы это клиенту, но не делал из этого слишком много.
  • HTML генерируется в бизнес-логике и будет кошмаром, чтобы разобраться . Хорошо, чувак, это одна из тех, где вы даете мне больше деталей. Если вы не используете MVC, это имеет тенденцию происходить ... но это не обязательно плохо ... это одна из тех вещей, где большинство программистов скажут: " Goto плохо, никогда не используйте его", но знаете что? Я использовал goto, где это имело смысл! Итак, вы уверены, что они не используют вспомогательные классы, которые совместно используют то же пространство имен, что и DLL бизнес-кода? Опять же, это не так сложно изолировать.
  • бизнес-логика распространяется по всему приложению, много дублирования и тупиковый код, который ничего не делает. , А также? Клиент просто просит вас изменить HTML / CSS. Почему вы заботитесь об этих проблемах вообще?
  • он продолжает генерировать исключения, которые заглушаются, поэтому сайт работает нормально . Опять очень расплывчато. Исключения являются нормальными в любых приложениях, поэтому в нашем коде есть предложения try / catch. Если они не всплывают в пользовательском интерфейсе и не портят пользовательский интерфейс (например, ненужное отображение HTTP 500), я не думаю, что это то, о чем вам следует также беспокоиться.

Короче говоря, я бы посоветовал вам пойти по большой дороге. Если вы думаете, что это не стоит вашего времени, и вы хотите переписать его за счет своего клиента, тогда уходите с работы. Серьезно, в конце, клиент платит за ваше время, чтобы заставить все это работать с наименьшим количеством $$$.

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

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

Декстер Легаспи
источник
-1

Конечно, атаки с использованием SQL-инъекций и другие функциональные недостатки в приложении имели приоритет, но вы также можете «продемонстрировать» плохое качество кода и практики. С помощью инструментов метрик кода вы можете четко продемонстрировать, насколько плох код и показать ему, сколько это будет стоить для любых будущих изменений и исправления ошибок. Я не знаком со средой .net, но уверен, что есть несколько, из которых можно выбрать.

Uberto
источник
Почему отрицание на это? Конечно, клиент может быть не технологичным, но метрики кода дают числа, и каждый может это понять. Особенно, если есть хорошо документированное, не слишком подробное объяснение того, что означают эти цифры
Мауг