Законно ли писать программу из процедур без in/out параметров, которые оперируют глобальными переменными?

Question

[palmage]

Добрый день!

Столкнулся с программой написанной на мой неопытный взгляд в странном стиле, а именно:

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

int a;
int b;
int result;
int c = 6;
void Click1() { 
   proc1();
   proc2();
   proc3(); 
}
void Click2() { 
   proc1();
   proc2();
   proc4(); 
}
void proc1() {a = 3;}
void proc2() {b = 5;}
void proc3() {result = a + b + c; Console.Write(result)}
void proc4() {result = a * b / c; Console.Write(result)}

Мне предстоит развивать это программу и меня интересует вопрос - допустимо ли писать программы таким образом или я должен приложить усилия, чтобы ее изменить или как минимум не повторять этот стиль в новых блоках кода?

На мой взгляд, программа должна должна выглядеть следующим образом:

int C = 6;
void Click1() { 
   proc1(int out a);
   proc2(int out b);
   proc3(a, b, C);
}
void Click2() { 
   proc1(int out a);
   proc2(int out b);
   proc4(a, b, C); 
}
void proc1(int out a) {a = 3;}
void proc2(int out b) {b = 5;}
void proc3(int a, int b, C) {
    int result;
    result = a + b + С;
    Console.Write(result)
}
void proc4(int a, int b, C) {
    int result;
    result = a * b / С;
    Console.Write(result)
}

Comments

[lz961]

можно я наброшу? В "оригинальной" программе процедура proc1 меняет глобальное состояние, и это может быть использовано в дальнейшем, в "правильно написанной" -- нет. Можете, конечно, всё это переписать используя паттерн "синглетон". В своих глазах это поможет вам превратиться из недоучки в грамотного кодера.

[Adamos]

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

[lz961]

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

[Adamos]

lz961, она меняет то, что в нее передано.
Если передана глобальная переменная - она и изменится.

[lz961]

Adamos, в вашем примере во втором случае одна глобальная переменная, я пишу о нём. Если вы снова сделаете a и b глобальными, у вас получатся процедуры, которые вызываются всегда с одним и тем же набором параметров, что тоже странно.

[Adamos]

lz961, это не мои примеры.
А если вы снова делаете переменные глобальными после рефакторинга - значит, с рефакторингом вы обделались.

[lz961]

Adamos,

Если передана глобальная переменная - она и изменится.

ваш текст?

[Adamos]

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

[lz961]

Adamos, я пишу про возможный слом функционала при таком грубом изменении кода.

[Adamos]

lz961, сдуру можно многое сломать.

Answer 1

[Akina]

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

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

Как я понимаю, программа эта выполняется локально и монопольно, вряд ли многопоточная, и даже допускаю, что не разрешающая одновременного запуска нескольких инстансов (хотя вот это уже совсем предположение). А потому не вижу никаких противнопоказаний к нормальному рефакторингу и получению в конечном итоге приложения, сохранившего весь функционал, но написанного "по науке".

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

Армянское Радио

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

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

Comments

[Николай Савельев]

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

[Максим К]

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

Answer 2

[Wataru]

За такой код, по идее, надо бить по рукам.

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

Очевидно, что писал это кто-то вообще без опыта или "переучившийся" на си с какого-то другого древнего языка.

Солидарен с другими отвечающими: если нет возможности это исправить - бегите.

Comments

[Adamos]

Console.Write() и in/out в параметрах - разве это С?

[palmage]

Adamos, Это С#. И я только начал его изучать. А программу на самом деле писали инженеры) Тем не менее работу нужно доделать, поэтому был бы благодарен за конкретные замечания и варианты устранения недостатков)

[Adamos]

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

Answer 3

[Николай Савельев]

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

Законно ли писать программу из процедур без in/out параметров, которые оперируют глобальными переменными?

Нет, не законно, согласно законам программирования DRY/SOLID и прочих. Но ты же о них не слышал, не так ли?

Answer 4

[Армянское Радио]

Правильным решением было бы забрать трудовую книжку.

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

4к строк - это еще можно перерефакторить за пару-тройку месяцев в обнимку с отладчиком. Мне в свое время досталось три проекта по 50к строк на Дельфи. С сущностями вроде Button72. Эх, были времена...

Comments

[Adamos]

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

[Армянское Радио]

Adamos, Опять же, когда на предприятии люди заняты не своим делом - это вызывает вопросы. Инженер может поставить задачу разработчику, может написать какой-то референс или пример. Но код из-под непрограммиста плохо живет как часть программного продукта (не важно, внутреннего или коммерческого) - именно из-за невозможности рефакторинга, отсутствия деления на модули и полного игнорирования SOLID и KISS.

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

[Adamos]

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

[Армянское Радио]

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

Присутствуют те же действующие лица - единственный класс, в который навалено все, имена пременных вида a, b, c, d (Потому что в тетратке у Чуня в формулах именно эти буквы), 0 штук комментариев - или другой вариант, любовно напиханные комментарии уровня Кэпа, но датировка их обычно относится к первому-второму дню разработки. Потом полезли баги и стало не до комментирования.

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

[Adamos]

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

Answer 5

[Filipp42]

Не знаю, в каком случае такой подход вообще применим (может, так пытались повысить производительность?), но я думаю, ещё пара тысяч строк кода, и программа станет абсолютно не читаемой, или по крайней мере поддерживать её будет очень трудно.
Рекомендую прочитать главы SICP об абстракции при помощи процедур.

Answer 6

[d-stream]

Законно.
Никто не запрещает даже спать на потолке и какать стоя. Но это неудобно и ноги постоянно пахнут.

Поэтому в рамках развития и поддержки программы - доводить её до более удобного в плане поддержки и развития состояния. Естественно изучив материалы про рефакторинг и его концепции (Кериевски, Мартин и др).