Я думал, я так себе программист, но меня посадили рефакторить код одного коллеги
От: Jack128  
Дата: 22.07.20 19:55
Оценка: :))) :))) :)
Из прекрасного:

if ((OutStyle and 34359738368) <> 0) and
        ((FormNum = 7) or (FormNum = 18)) and
        (((OutStyle and 70368744177664) = 0) or
          (((OutStyle and 70368744177664) <> 0) and (OneAct))) then
        NRow := BuildItogTwoPriceLevels(OO, true, false, ACell, -1, NRow, FormNum, OutStyle, nil, {[]}DocOptions, false)


if Trim(FName)='Пустой идентификатор' then Result := aNone
 else if Trim(FName)='Учитываемая позиция' then Result := aActivePos
 else if Trim(FName)='Номер позиции по смете' then Result := aNpp
 else if Trim(FName)='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
  // далее 900 таких же строк



// классы TSmeta/TCustomObjSmeta/TCennik - конкретные классы, все наследники базового класса, у которого как раз объявлено свойство DocSignatures
function GetSignByID(ToAct: boolean; ASmeta: TSmeta; ObjSm: TCustomObjSmeta; ACennik: TCennik; AResSummary: TResSummary;
AProjectSmeta: TProjectSmeta; AWV: TWorksVolumes; ADS: TDocSummary; TC: TTranspCalc; CaptionSign: boolean; IDS: integer; IDA: integer): string;
begin
....
if ObjSm<>nil then SSS := TGsDocSignatures(ObjSm.DocSignatures);
if ACennik<>nil then SSS := TGsDocSignatures(ACennik.DocSignatures);
if AResSummary<>nil then SSS := TGsDocSignatures(AResSummary.DocSignatures);
if AProjectSmeta<>nil then SSS := TGsDocSignatures(AProjectSmeta.DocSignatures);
if AWV<>nil then SSS := TGsDocSignatures(AWV.DocSignatures);
if ADS<>nil then SSS := TGsDocSignatures(ADS.DocSignatures);
if TC<>nil then SSS := TGsDocSignatures(TC.DocSignatures);

...
end;


if
(NeedResourceType(ARow) or ((RootType = rrtDeleted) and (exopDelRes in DocOptions)) and
((FormNum = 20) or (FormNum = 21) or (FormNum = 13))) or
(TSmetaResourceTypeEx(ResourceType) = smrtEquip) or
((FormNum <> 20) and (FormNum <> 21) and (FormNum <> 13))
then
if (((FormNum = 20) or (FormNum = 21) or (FormNum = 13)) and
// !
(((ReportString[sdmQtTot] = '') and
((OutStyle and 2097152) <> 0)) or
(ReportString[sdmQtTot] <> ''))) or
((FormNum <> 20) and (FormNum <> 21) and (FormNum <> 13))
then // !
begin
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: L.K. Марс  
Дата: 22.07.20 20:04
Оценка: +1 :)
J>
J>if Trim(FName)='Пустой идентификатор' then Result := aNone
J> else if Trim(FName)='Учитываемая позиция' then Result := aActivePos
J> else if Trim(FName)='Номер позиции по смете' then Result := aNpp
J> else if Trim(FName)='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
J>  // далее 900 таких же строк
J>


Это код коллеги? Или это код десятка предыдущих коллег, первый из которых работал 10-20 лет назад?

И я подозреваю, что началось всё с небольшой софтины, в которой было не 900, а 4-5 строк. "Пока что сделаю быстро так, а то времени нет возиться со всякими id".

А потом строки наращивали. Почему наращивали? Чтобы не рефакторить. А то вдруг сломается. Работает — не трогаем.
Re[2]: Я думал, я так себе программист, но меня посадили реф
От: Jack128  
Дата: 22.07.20 20:06
Оценка:
Здравствуйте, L.K., Вы писали:

LK>Это код коллеги? Или это код десятка предыдущих коллег, первый из которых работал 10-20 лет назад?


Это код одного человека.

И нет, всё начилось с достовской софтины, в которой были десятки тысяч строк.
Отредактировано 22.07.2020 20:08 Jack128 . Предыдущая версия .
Re: Я думал, я так себе программист, но меня посадили рефакт
От: andyp  
Дата: 22.07.20 20:08
Оценка: +9 :))) :))
Здравствуйте, Jack128, Вы писали:

J>Из прекрасного:


Оставь как есть. Только испортишь. Константы особенно понравились: 34359738368. Чел знает толк в таких вещах.
Отредактировано 22.07.2020 20:10 andyp . Предыдущая версия .
Re[2]: Я думал, я так себе программист, но меня посадили рефакт
От: Jack128  
Дата: 22.07.20 20:18
Оценка:
Здравствуйте, andyp, Вы писали:

A>Здравствуйте, Jack128, Вы писали:


J>>Из прекрасного:


A>Оставь как есть. Только испортишь. Константы особенно понравились: 34359738368. Чел знает толк в таких вещах.


если бы дело в драйверах или подобной хрени. Это код выгрузки документа в Excel
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: bnk СССР http://unmanagedvisio.com/
Дата: 22.07.20 20:20
Оценка: :))) :)
Здравствуйте, Jack128, Вы писали:

J>Из прекрасного:


IMHO код нормальный,

  а вот контора не очень
Re[3]: Я думал, я так себе программист, но меня посадили рефакт
От: andyp  
Дата: 22.07.20 20:24
Оценка: 1 (1) +3 :))) :))) :))) :))) :))) :))) :)
Здравствуйте, Jack128, Вы писали:


A>>Оставь как есть. Только испортишь. Константы особенно понравились: 34359738368. Чел знает толк в таких вещах.


J>если бы дело в драйверах или подобной хрени. Это код выгрузки документа в Excel


Не важно что это, потому что это просто красиво. Утилитарист какой-нибудь написал бы $800000000 и все испортил
Re[4]: Я думал, я так себе программист, но меня посадили рефакт
От: Jack128  
Дата: 22.07.20 21:44
Оценка:
Здравствуйте, andyp, Вы писали:

A>Не важно что это, потому что это просто красиво. Утилитарист какой-нибудь написал бы $800000000 и все испортил


Ох, ты не представляешь, как испортил бы, особенно, если учесть, что в Delphi (а код на нем) — еще есть и специальный тип и поддержка языка для работы с битовами масками.
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: зиг Украина  
Дата: 22.07.20 22:16
Оценка:
Здравствуйте, Jack128, Вы писали:

J>Из

а кто это писал такое, можешь описать человека? не фио, а так, типа черты характера, возраст, семейное положение.
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: CreatorCray  
Дата: 22.07.20 22:32
Оценка: +1
Здравствуйте, Jack128, Вы писали:


J>
J>if ((OutStyle and 34359738368) <> 0) and
J>        ((FormNum = 7) or (FormNum = 18)) and
J>        (((OutStyle and 70368744177664) = 0) or
J>          (((OutStyle and 70368744177664) <> 0) and (OneAct))) then
J>        NRow := BuildItogTwoPriceLevels(OO, true, false, ACell, -1, NRow, FormNum, OutStyle, nil, {[]}DocOptions, false)
J>


Человек-декомпилятор какой то.

J>
J>  // далее 900 таких же строк
J>


... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re[3]: Я думал, я так себе программист, но меня посадили рефакт
От: CreatorCray  
Дата: 22.07.20 22:32
Оценка: :))
Здравствуйте, Jack128, Вы писали:

>если бы дело в драйверах или подобной хрени.

В драйверах суровые бородатые дядьки за такое сразу роняют барабанный принтер на руки автору.
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re[5]: Я думал, я так себе программист, но меня посадили рефакт
От: CreatorCray  
Дата: 22.07.20 22:32
Оценка:
Здравствуйте, Jack128, Вы писали:

J>Ох, ты не представляешь, как испортил бы, особенно, если учесть, что в Delphi (а код на нем) — еще есть и специальный тип и поддержка языка для работы с битовами масками.


Вот видишь! Получилась бы новомодная бездуховщина. А так — тепло и лампово, как деды завещали!
... << RSDN@Home 1.3.110 alpha 5 rev. 62>>
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: Rhino СССР  
Дата: 22.07.20 23:40
Оценка:
Здравствуйте, Jack128, Вы писали:

J>Из прекрасного:

Это ты ещё конфигурации от 1С не видел
... << RSDN@Home 1.0.0 alpha 5 rev. 0>>
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: varenikAA  
Дата: 23.07.20 00:45
Оценка:
Здравствуйте, Jack128, Вы писали:

J>Из прекрасного:


J>[pascal]

J>if ((OutStyle and 34359738368) <> 0) and



Это какая версия?
В первом случае непонятно, это метод класса или функция?
Во втором, да можно было словарик прикрутить.
☭ ✊ В мире нет ничего, кроме движущейся материи.
Re[2]: Я думал, я так себе программист, но меня посадили реф
От: bnk СССР http://unmanagedvisio.com/
Дата: 23.07.20 03:21
Оценка: +1
Здравствуйте, varenikAA, Вы писали:

AA>Это какая версия?

AA>В первом случае непонятно, это метод класса или функция?
AA>Во втором, да можно было словарик прикрутить.

Со словариком стало бы только хуже, IMHO, пришлось бы вместо одного в двух местах править при изменении структуры файла Excel, с которым этот код работает.
А так как есть IMHO удобнее — вся бизнес-логика сконцентрирована в одном месте.

Вот длинным константам и номерам форм у меня как-то с трудом находится оправдание. Должна же быть тут какая-то логика?

Может он не знал что там можно шестнадцатеричные числа писать? Вряд ли.
Или может раньше нельзя было (из-за какого-нибудь бага в компиляторе например)?

Номера форм, возможно в организации все формы (бумажные) имеют номер.
Типа, "форма номер 61Ф". Тогда в номерах появляется смысл.


В общем, я бы это древнее зло не трогал, во избежание
Отредактировано 23.07.2020 3:43 bnk . Предыдущая версия . Еще …
Отредактировано 23.07.2020 3:23 bnk . Предыдущая версия .
Отредактировано 23.07.2020 3:22 bnk . Предыдущая версия .
Re[2]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: VerHanna Польша  
Дата: 23.07.20 06:18
Оценка:
Здравствуйте, зиг, Вы писали:

зиг>Здравствуйте, Jack128, Вы писали:


J>>Из

зиг>а кто это писал такое, можешь описать человека? не фио, а так, типа черты характера, возраст, семейное положение.

У нас есть такой на конторе товарищ.
Кстати,тоже пишет на делфях.
Самой конторе 25+ лет, софту еще больше, миллионы строк кода.
Челу 65+ лет, живет в новой зеландии, наверное уже дедушка. Программит больше для души, но когда чтото фиксит, то падает вокруг все подряд. Держат на конторе в том числе и по причине выслуги лет.
"Ну дела, — подумал Лось, —
Не хотелось.
А пришлось". (c)
Re[2]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: Mr.Delphist  
Дата: 23.07.20 09:34
Оценка:
Здравствуйте, CreatorCray, Вы писали:

CC>Человек-декомпилятор какой то.


Вот у меня тоже ощущение, что сырцы когда-то прос... потеряли, и затем из бинарника ревертнули. А дельше — нет времени объяснять, пилите дальше.
Re[3]: Я думал, я так себе программист, но меня посадили реф
От: Sealcon190 Соломоновы острова  
Дата: 23.07.20 09:53
Оценка:
Здравствуйте, bnk, Вы писали:

bnk>Может он не знал что там можно шестнадцатеричные числа писать? Вряд ли.

bnk>Или может раньше нельзя было (из-за какого-нибудь бага в компиляторе например)?

Даже если нельзя писать числа в хексе, такую бороду всегда можно спрятать за константу с красивым названием.
Re[2]: Я думал, я так себе программист, но меня посадили рефакт
От: Философ Ад http://vk.com/id10256428
Дата: 23.07.20 13:27
Оценка:
Здравствуйте, andyp, Вы писали:

A>Оставь как есть. Только испортишь. Константы особенно понравились: 34359738368. Чел знает толк в таких вещах.


Нет, не знает: это копипаста из интернетов. Т.е. сначала из интернетов, а потом из угла в угол в коде.
Всё сказанное выше — личное мнение, если не указано обратное.
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: Sharov Россия  
Дата: 23.07.20 14:18
Оценка:
Здравствуйте, Jack128, Вы писали:

J>
J>if Trim(FName)='Пустой идентификатор' then Result := aNone
J> else if Trim(FName)='Учитываемая позиция' then Result := aActivePos
J> else if Trim(FName)='Номер позиции по смете' then Result := aNpp
J> else if Trim(FName)='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
J>  // далее 900 таких же строк
J>


Тут что не так, за исключением того, что Trim, возможно, будет вызываться 900 раз?
Кодом людям нужно помогать!
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: e-Xecutor Россия  
Дата: 23.07.20 15:07
Оценка: :)
Здравствуйте, Jack128, Вы писали:

Я недавно видел свеженаписанный код. В нём КАЖДАЯ строка была с комментарием на русском. Но! Частенько содержимое комментария не совпадало с действием в коде.
Ну и апофеозом была функция с 18 вложенными if. 18 Карл! Восемнадцать! 0x12!
Re[2]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: alzt  
Дата: 23.07.20 17:34
Оценка:
Здравствуйте, L.K., Вы писали:


J>>
J>>if Trim(FName)='Пустой идентификатор' then Result := aNone
J>> else if Trim(FName)='Учитываемая позиция' then Result := aActivePos
J>> else if Trim(FName)='Номер позиции по смете' then Result := aNpp
J>> else if Trim(FName)='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
J>>  // далее 900 таких же строк
J>>


LK>И я подозреваю, что началось всё с небольшой софтины, в которой было не 900, а 4-5 строк. "Пока что сделаю быстро так, а то времени нет возиться со всякими id".


Зачем id, потом sed натравливаешь, и всё работает.
Может у коллеги даже собственный софт есть по модификации своего кода.
Re: Я думал, я так себе программист, но меня посадили рефакторить код одного кол
От: bpost  
Дата: 24.07.20 05:19
Оценка:
Только не показывайте это дядюшке Бобу — старину хватит инфаркт
Re[2]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: Socrat Россия  
Дата: 24.07.20 06:47
Оценка:
Здравствуйте, Sharov, Вы писали:

S>Тут что не так, за исключением того, что Trim, возможно, будет вызываться 900 раз?


Я тоже особого криминала не вижу. Хотя, насколько я помню, в паскале case работает со строками.
Re[2]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: Denwer Россия  
Дата: 24.07.20 09:15
Оценка:
Здравствуйте, Sharov, Вы писали:

S>Здравствуйте, Jack128, Вы писали:


J>>
J>>if Trim(FName)='Пустой идентификатор' then Result := aNone
J>> else if Trim(FName)='Учитываемая позиция' then Result := aActivePos
J>> else if Trim(FName)='Номер позиции по смете' then Result := aNpp
J>> else if Trim(FName)='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
J>>  // далее 900 таких же строк
J>>


S>Тут что не так, за исключением того, что Trim, возможно, будет вызываться 900 раз?


Если таких строк уже 900, то они не должны быть в самом коде. Вынести в какой нибудь внешний текстовый файл-конфиг и при старте зачитывать в дерево для быстрого поиска.
Re[3]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: bnk СССР http://unmanagedvisio.com/
Дата: 24.07.20 09:27
Оценка:
Здравствуйте, Denwer, Вы писали:

J>>>
J>>>if Trim(FName)='Пустой идентификатор' then Result := aNone
J>>> else if Trim(FName)='Учитываемая позиция' then Result := aActivePos
J>>> else if Trim(FName)='Номер позиции по смете' then Result := aNpp
J>>> else if Trim(FName)='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
J>>>  // далее 900 таких же строк
J>>>


S>>Тут что не так, за исключением того, что Trim, возможно, будет вызываться 900 раз?


D>Если таких строк уже 900, то они не должны быть в самом коде. Вынести в какой нибудь внешний текстовый файл-конфиг и при старте зачитывать в дерево для быстрого поиска.


Ужас какой. Зачем!? Это же работа с Excel, логику там наверняка правят не по разу.
Предлагаешь теперь десять файлов править вместо одного, да еще и фреймворк писать для чтения конфига?!
Re[2]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: Aniskin  
Дата: 24.07.20 09:33
Оценка: 1 (1)
Здравствуйте, Sharov, Вы писали:

S>Здравствуйте, Jack128, Вы писали:


J>>
J>>if Trim(FName)='Пустой идентификатор' then Result := aNone
J>> else if Trim(FName)='Учитываемая позиция' then Result := aActivePos
J>> else if Trim(FName)='Номер позиции по смете' then Result := aNpp
J>> else if Trim(FName)='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
J>>  // далее 900 таких же строк
J>>


S>Тут что не так, за исключением того, что Trim, возможно, будет вызываться 900 раз?


Я бы сделал примерно так:

type
  TSomeType = (aNone, aActivePos, aNpp, aNppImp);

  TStrToSomeTypeRec = record
    Str: string;
    SomeType: TSomeType;
  end;

const
  StrToTypeArray: array[0..3] of TStrToSomeTypeRec = (
    (Str: 'Пустой идентификатор'; SomeType: aNone),
    (Str: 'Учитываемая позиция'; SomeType: aActivePos),
    (Str: 'Номер позиции по смете'; SomeType: aNpp),
    (Str: 'Номер позиции по порядку (в актах выполненных работ)'; SomeType: aNppImp));

function StrToSomeType(Str: string): TSomeType;
var
  Index: Integer;
begin
  Str := Trim(Str);
  for Index := Low(StrToTypeArray) to High(StrToTypeArray) do
    if Str = StrToTypeArray[Index].Str then
      Exit(StrToTypeArray[Index].SomeType);
  raise Exception.Create('String not found');
end;
Re[3]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: necr0n0mic0n  
Дата: 24.07.20 11:00
Оценка:
Здравствуйте, Socrat, Вы писали:

S>Здравствуйте, Sharov, Вы писали:


S>>Тут что не так, за исключением того, что Trim, возможно, будет вызываться 900 раз?


S>Я тоже особого криминала не вижу. Хотя, насколько я помню, в паскале case работает со строками.


не работает. только с ordinal types: char, byte, integer, dword etc.

ну и вызывать Trim() 900 раз....

неужели так сложно было

s := Trim(FName);
if s='Пустой идентификатор' then Result := aNone
else if s='Учитываемая позиция' then Result := aActivePos
else if s='Номер позиции по смете' then Result := aNpp
else if s='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
Re[3]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: Ops Россия  
Дата: 24.07.20 12:01
Оценка:
Здравствуйте, Aniskin, Вы писали:

J>>>
A>  StrToTypeArray: array[0..3] of TStrToSomeTypeRec = (


А вот это вот 3 там можно как-то автоматом вывести, чтобы не считать эти строки?
Переубедить Вас, к сожалению, мне не удастся, поэтому сразу перейду к оскорблениям.
Re[2]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: зиг Украина  
Дата: 24.07.20 12:10
Оценка: +1
Здравствуйте, Sharov, Вы писали:

S>Здравствуйте, Jack128, Вы писали:


J>>
J>>if Trim(FName)='Пустой идентификатор' then Result := aNone
J>> else if Trim(FName)='Учитываемая позиция' then Result := aActivePos
J>> else if Trim(FName)='Номер позиции по смете' then Result := aNpp
J>> else if Trim(FName)='Номер позиции по порядку (в актах выполненных работ)' then Result := aNppImp
J>>  // далее 900 таких же строк
J>>


S>Тут что не так, за исключением того, что Trim, возможно, будет вызываться 900 раз?



дизайн приложения очень фиговый. это означит что эти строки не только здесь фигрурируют но и где-то еще, и если кто-нибудь попросит немного поменять текст — замудохаешься его менять в двух (я подозреваю что при описанном стиле кодирования то и в трех и в четырх и т.д.) местах.
т.е. вместо этих магических строковых констант должен быть енум, типа EMPTY_ID("Пустой идентификатор"), ...
c enumами можно swtich/case использовать. правда я не знаю есть ли енумы в дельфи или что тут за язык
Re[4]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: Aniskin  
Дата: 24.07.20 12:30
Оценка:
Здравствуйте, Ops, Вы писали:

Ops>А вот это вот 3 там можно как-то автоматом вывести, чтобы не считать эти строки?


Такого нет, но компилятор подскажет: E2072 Number of elements (900) differs from declaration (4)
Re[3]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: Socrat Россия  
Дата: 24.07.20 12:40
Оценка:
Здравствуйте, зиг, Вы писали:

зиг>дизайн приложения очень фиговый. это означит что эти строки не только здесь фигрурируют но и где-то еще


Не факт. Представь, что тебе приходит информация от сторонней организации в виде xml-файла, в котором каждое поле имеет свое название. Причем, организаций может быть не одна. И формат может меняться раз в несколько лет. А этот код переводит название этих полей в собственный enum.
Re[3]: Я думал, я так себе программист, но меня посадили реф
От: bnk СССР http://unmanagedvisio.com/
Дата: 24.07.20 22:27
Оценка:
Здравствуйте, зиг, Вы писали:

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

зиг>т.е. вместо этих магических строковых констант должен быть енум, типа EMPTY_ID("Пустой идентификатор"), ...
зиг>c enumами можно swtich/case использовать. правда я не знаю есть ли енумы в дельфи или что тут за язык

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

А вот трим, да, похоже на копи-пасту. Но может там где-то важны пробелы в начале-конце, я х.з.
Отредактировано 24.07.2020 22:37 bnk . Предыдущая версия . Еще …
Отредактировано 24.07.2020 22:37 bnk . Предыдущая версия .
Отредактировано 24.07.2020 22:29 bnk . Предыдущая версия .
Отредактировано 24.07.2020 22:29 bnk . Предыдущая версия .
Re[4]: Я думал, я так себе программист, но меня посадили реф
От: зиг Украина  
Дата: 24.07.20 22:52
Оценка:
Здравствуйте, bnk, Вы писали:

bnk>Здравствуйте, зиг, Вы писали:


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

зиг>>т.е. вместо этих магических строковых констант должен быть енум, типа EMPTY_ID("Пустой идентификатор"), ...
зиг>>c enumами можно swtich/case использовать. правда я не знаю есть ли енумы в дельфи или что тут за язык

bnk>Сомневаюсь что так было бы лучше. Так еще и идентификаторы эти поддерживать пришлось бы.

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

bnk>Если предположить, что этот код добывает данные из файла Excel (или нескольких), который некая сердобольная тетенька верстает, как ей заблагорассудится,

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

bnk>Поддержка повторного использования, или какая-то оптимизация тут IMHO абсолютно не нужна и даже будет вредна.

ок, моё ИМХО код очевидно ужасен этими строковыми константами на основе которых что-то там хитрое определяется.
Re[4]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: зиг Украина  
Дата: 24.07.20 23:09
Оценка:
Здравствуйте, Socrat, Вы писали:

S>Здравствуйте, зиг, Вы писали:


зиг>>дизайн приложения очень фиговый. это означит что эти строки не только здесь фигрурируют но и где-то еще


S>Не факт. Представь, что тебе приходит информация от сторонней организации в виде xml-файла, в котором каждое поле имеет свое название. Причем, организаций может быть не одна. И формат может меняться раз в несколько лет. А этот код переводит название этих полей в собственный enum.


тогда я бы все равно сделала по-другому. у меня все равно был бы енум, и чтоб получить его инстанс я бы вызывала MyEnum.valueOf(stringReceivedFromXML) а потом был бы свитч по енуму
Re[5]: Я думал, я так себе программист, но меня посадили реф
От: bnk СССР http://unmanagedvisio.com/
Дата: 24.07.20 23:29
Оценка:
Здравствуйте, зиг, Вы писали:

bnk>>Сомневаюсь что так было бы лучше. Так еще и идентификаторы эти поддерживать пришлось бы.

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

Непонятно, зачем лишняя сущность (ID)? Например, тот же код на javascript [для уменьшения синтаксического оверхеда (c)], зачем тут какие-то ID?
Подразумевается, что идентификаторы используются ровно в одном месте (здесь), для "адресации" колонки в Excel.

Result = {

  'Пустой идентификатор': aNone,
  'Учитываемая позиция': aActivePos,
  'Номер позиции по смете': aNpp,
  'Номер позиции по порядку (в актах выполненных работ)': aNppImp

}[FName.trim()];

Кстати идентификатор прикольный раньше не заметил: BuildItogTwoPriceLevels. Похоже код реально бронтозавр писал, хватит его оправдывать
Отредактировано 24.07.2020 23:40 bnk . Предыдущая версия . Еще …
Отредактировано 24.07.2020 23:38 bnk . Предыдущая версия .
Re[5]: Я думал, я так себе программист, но меня посадили рефакторить код одного
От: Socrat Россия  
Дата: 27.07.20 07:41
Оценка:
Здравствуйте, зиг, Вы писали:

зиг>тогда я бы все равно сделала по-другому. у меня все равно был бы енум, и чтоб получить его инстанс я бы вызывала MyEnum.valueOf(stringReceivedFromXML) а потом был бы свитч по енуму


Спорное решение. Код выше прост и очевиден. А это немаловажное преимущество.
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.