Как вам более понятно?

← →
sniknik © (2015-05-19 11:37) [0]

Просто разбираю, в очередной раз, чужой код, вроде бы неплохой и даже без ошибок (жалоб на работу программы нет), но в виде "стена текста" (первый вариант), куча лишних действий (ИМХО) формирование строк всегда через Format, запросы вынесены в константы (+ поэтапное внесение... т.е. константу в переменную, а переменную после в CommandText), условия в отдельных функциях хотя можно сделать простое сравнение и т.д. Такое впечатление, что человеку за килобайты платят...
Наглядный пример ниже, + вариант как бы сделал то же самое я. Как лучше (правильней/эстетичней/...) по вашему? (просто мне к примеру читать первый вариант тяжело, иногда приходится переформатировать чтобы просто понять что тут делается)
sBody := Format("ID перевода: %s", [aID]); sBody := sBody + #13#10; sBody := sBody + Format(#13#10"Номер перевода: %s", [aTRN]); sBody := sBody + Format(#13#10"Код точки: %s", [aPP_CODE]); sBody := sBody + Format(#13#10"Дата: %s", [aDT_FILE]); sBody := sBody + #13#10; sBody := sBody + Format(#13#10"Номер претензии: %s", [IfThen(bUnknown, "", IntToStr(idWork))]); sBody := sBody + Format(#13#10"%s", [sActDescr]); sBody := sBody + Format(#13#10"Комментарий: %s", [aComment]); sBody := sBody + Format(#13#10"Имя клиента: %s", [aName]); sBody := sBody + Format(#13#10"Телефон: %s", [aPhone]); sBody := sBody + Format(#13#10"Почта: %s", [aMail]); sBody := "ID перевода: " + aID +#13#10+ #13#10+ "Номер перевода: " + aTRN +#13#10+ "Код точки: " + aPP_CODE+#13#10+ "Дата: " + aDT_FILE+#13#10+ #13#10+ "Номер претензии: "+ IfThen(bUnknown, "", IntToStr(idWork)) +#13#10+ [sActDescr] +#13#10+ "Комментарий: " + aComment+#13#10+ "Имя клиента: " + aName +#13#10+ "Телефон: " + aPhone +#13#10+ "Почта: " + aMail;

← →
Dimka Maslov © (2015-05-19 11:39) [1]

Предпочитаю вариант №1. Так лучше во время отладки. А еще я длинные выражения разбиваю на мелкие. Опять же, чтобы в отладчике понять, где именно происходит бяка.

← →
Kerk © (2015-05-19 11:51) [2]

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

← →
sniknik © (2015-05-19 12:02) [3]

> если часто его вызывать.
Тут не часто, тут всегда. Просто стиль такой у человека. В циклах, нет, без разницы.

← →
Игорь Шевченко © (2015-05-19 12:02) [4]

Второй, но без IfThen

← →
sniknik © (2015-05-19 12:03) [5]

Кстати насчет производительности говорил... ответ "оптимизатор все к одному приведет". Может даже это так и есть.

← →
Игорь Шевченко © (2015-05-19 12:04) [6]

> Просто стиль такой у человека

Уволить нельзя ? Я бы за одни имена идентификаторов лишал бы права на занятие программистской деятельностью.

← →
Игорь Шевченко © (2015-05-19 12:04) [7]

> "оптимизатор все к одному приведет"

Лучший оптимизатор - голова программиста

← →
sniknik © (2015-05-19 12:06) [8]

> Второй, но без IfThen
Это из его кода, переделал "как есть" не меняя логики, про функции я отдельно написал. В принципе тоже против.

← →
DVM © (2015-05-19 12:09) [9]

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

← →
sniknik © (2015-05-19 12:10) [10]

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

← →
Игорь Шевченко © (2015-05-19 12:16) [11]

> Один из лучших...

Пусть почитает на досуге:
http://edn.embarcadero.com/article/10280

← →
кгшзх © (2015-05-19 12:16) [12]

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

← →
sniknik © (2015-05-19 12:25) [13]

> Второй, но без IfThen
О блин, а условие то и не работает... т.е. там еще до этого проверка на
bUnknown := StrToIntDef(aID, -2) < 0; // Неопределенная жалоба if not bUnknown then begin
Т.е. по смыслу, если жалоба не найдена/не определена (код -1, -2) то и письмо не посылается, внутренняя ошибка, внутри подобного условия было бы достаточно (у определенной номер есть всегда)
"Номер претензии: "+ IntToStr(idWork) +#13#10+

> если нужно выполнить такой же точно запрос
Это не запрос, это тело письма. Про запросы в тексте было "до кучи".

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

← →
Pavia © (2015-05-19 12:30) [14]

> sBody := sBody + Format

Повторяющийся код надо выкинуть. Но первый вариант форматирования выглядит по симпатичнее.

Format - разумно использовать если код предпологает изменения. На вскидку тут вероятность этого мола.

> условия в отдельных функциях хотя можно сделать простое
> сравнение и т.д.

Хотелось бы посмотреть насколько это похоже на мой стиль.
Чем больше функций тем легче писать и поддерживать код.

> запросы вынесены в константы (+ поэтапное внесение... т.
> е. константу в переменную, а переменную после в CommandText

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

← →
Kerk © (2015-05-19 12:31) [15]

> sniknik © (19.05.15 12:03) [5]
>
> Кстати насчет производительности говорил... ответ "оптимизатор
> все к одному приведет". Может даже это так и есть.

Вряд ли оптимизатор сможет объединить десять вызовов функции Format в один.

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

← →
Ega23 © (2015-05-19 12:46) [16]

Второй. Сам так пишу, с таким же форматированием. И даже так же табуляцию порой использую.
А вот IfThen заменил бы на функцию, может быть даже на вложенную.

← →
sniknik © (2015-05-19 12:52) [17]

> Повторяющийся код надо выкинуть. Но первый вариант форматирования выглядит по симпатичнее.
Если там будет всего 1 формат... куда ни шло, но читать все одно приятного мало, будет куча %s в тексте, и если он большой (ну на 20, 50 переменных к примеру) то попробуй еще определи, что за переменная с ним согласуется... (единственный выход вижу это синтетические значения переменных типа "1" ... "20" и текст формируемый смотреть... не удобно при разборе/поиске ошибок)

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

← →
Pavia © (2015-05-19 13:18) [18]

> > Повторяющийся код надо выкинуть. Но первый вариант форматирования
> выглядит по симпатичнее.
> Если там будет всего 1 формат... куда ни шло, но читать
> все одно приятного мало, будет куча %s в тексте, и если
> он большой (ну на 20, 50 переменных к примеру) то попробуй
> еще определи, что за переменная с ним согласуется... (единственный
> выход вижу это синтетические значения переменных типа "1"
> ... "20" и текст формируемый смотреть... не удобно при разборе/поиске
> ошибок)

Это вы просто не читали PhP для проффесионалов. Делается шаблон и пасрсер.
Шаблон хранится в фаайле и имеет вид:

Номер перевода: %aTRN
Код точки: %aPP_CODE
Дата: %aDT_FILE

Гораздо нагляднее. Вопросом скорости при этом особо внимания не уеляли.

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

Не преблема. Решается вполне детерменированно. Делается шаблон, потом парсер. После разбора парсерером шаблона на выходе имеем структуру типа массива. Где на каждую переменную заводится геттер. Динамически связывается с нужными методами, объекта. Набор переменных и их геттеров можно передать при инициализации парсера. Код будет рабоать очень быстро. Только кода будет очень много!

← →
sniknik © (2015-05-19 13:37) [19]

> Номер перевода: %aTRN
> Код точки: %aPP_CODE
> Дата: %aDT_FILE
> Гораздо нагляднее.
Это да, только не %, а $ в PhP вроде... и сравнение не корректное, т.к. PhP это фактически код в тексте, а дельфя текст в коде... там то будет не рядом, а

Номер перевода: %s
Код точки: %s
Дата: %
... еще 47 строк текста
и в конце
[aTRN, aPP_CODE, aDT_FILE ... всего 50 переменных]

Разница однако.

← →
Дмитрий С © (2015-05-19 13:43) [20]

Голосую за второй.

В Intellij Idea есть такая фишка как Edit ??? Fragment, которая позволяет редактировать подобную конкатенацию целым кусочком (в отдельном окне) - очень удобная штука. Глядишь и в Delphi IDE такое появится когда-нибудь.

← →
Pavia © (2015-05-19 13:46) [21]

Вы не поняли.

> Это да, только не %, а $ в PhP вроде... и сравнение не корректное,
> т.к. PhP это фактически код в тексте, а дельфя текст в
> коде... там то будет не рядом, а

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

← →
Inovet © (2015-05-19 14:18) [22]

Второй вкусней.

← →
sniknik © (2015-05-19 14:22) [23]

> Вы не поняли.
Да не понял, не догадался, что мне написать PhP предлагают, пусть частично... но смысл? Это кому то изменит стиль? Или просто даст еще одну возможность писать запутано? ИМХО последнее.

← →
Kerk © (2015-05-19 14:51) [24]

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

← →
Pavia © (2015-05-19 15:11) [25]

> Писать целый движок шаблонов ради логгирования - это перебор
> по-моему.

Ну не сразу же. Вначале пару функций по 50 строк. Зато потом можно наращивать. До полноценного.
А как вы протоколируете(анг. логируете) без движка? Ведь иначе либо протокол не полный будет либо программа будет тормозить.

← →
ухты © (2015-05-19 15:24) [26]

второй с Format

← →
ухты © (2015-05-19 15:25) [27]

т.е. 1 строка 1 формат и никаких "+"

← →
Дмитрий С © (2015-05-19 15:34) [28]

> ухты © (19.05.15 15:25) [27]

А как ты визуально разобьешь строку?

зачем ее бить?

Второй однозначно. Я бы еще от #13#10 избавился визуально, портят наглядность. И чтобы каждая логически законченная строка в одной строке.

а зачем нужен Format?

по большому счету строку надо в ресурсы загнать
если ее приходиться частенько редактировать то тут уже не 1 и не 2 не годны :)

В C# 6.0 ввели новую возможность:

Интерполяция строк

Каждый день нам приходится сталкиваться с конкатенацией строк. Кто-то в основном использует оператор “+”, кто-то — метод string.Format(). Мне лично по душе string.Format(). Но проблемы с ним всем известны: при слишком большом количестве параметров тяжело понимать, что означают каждое число – {1}, {2}, {3}. В C# 6.0 придумали новую возможность, которая должна объединить достоинства обоих методов.

Раньше:

name = string.Format("Employee name is {0}, located at {1}", emp.FirstName, emp.Location);

Теперь:

name = $"Employee name is {emp.FirstName}, located at {emp.Location}";

Так же можно использовать условия:

name = $"Employee name is {emp.FirstName}, located at {emp.Location}. Age of employee is {(emp.Age > 0) ? emp.Age.ToString() : "N/A"}";

http://habrahabr.ru/post/249555/

> [22] Inovet © (19.05.15 14:18)
> Второй вкусней.

Но, блин, предпочитаю третий, когда строка с подстановками и функция Формат. Да, хуже, но универсальнее, да и строку в исходнике можно оформить в более адекватном виде.

> Игорь Шевченко © (19.05.15 12:16) [11]
> Пусть почитает на досуге:http://edn.embarcadero.com/article/10280

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

Потому что Игорь Шевченко всегда прав. ☺

>RWolf © (19.05.15 17:38) [36]
>Почему все разработчики должны следовать стилю, принятому в своё время в Борланд?

Это не стиль Борланд. Это наиболее понятный и читаемый стиль как таковой. Именно поэтому его и использовали в Борланд. И другим советуют.

> Это не стиль Борланд.

>> http://edn.embarcadero.com/article/10280
>>> Delphi is created in California, so we discourage the use of notation ...

- а мне венгры таки ближе...

Первый, но измененный.

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

const MyString = "ID перевода: %s" +#13#10+ #13#10+ // aID "Номер перевода: %s" +#13#10+ // aTRN "Код точки: %s" +#13#10; итд //aPP_CODE sBody := Format(MyString, [aID, aTRN, aPP_CODE]);

Но если код очень часто вызывается, то второй.

Как вам более понятно? Найти похожие ветки