Форум: "Прочее";
Текущий архив: 2016.01.24;
Скачать: [xml.tar.bz2];
ВнизКак вам более понятно? Найти похожие ветки
← →
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]
А как ты визуально разобьешь строку?
← →
ухты © (2015-05-19 15:44) [29]зачем ее бить?
← →
Сергей Суровцев © (2015-05-19 15:53) [30]Второй однозначно. Я бы еще от #13#10 избавился визуально, портят наглядность. И чтобы каждая логически законченная строка в одной строке.
← →
картман © (2015-05-19 16:00) [31]а зачем нужен Format?
← →
ухты © (2015-05-19 16:04) [32]по большому счету строку надо в ресурсы загнать
если ее приходиться частенько редактировать то тут уже не 1 и не 2 не годны :)
← →
euru © (2015-05-19 16:13) [33]В 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/
← →
Eraser © (2015-05-19 17:00) [34]
> euru © (19.05.15 16:13) [33]
хорошая возможность, украли из php, кстати )
← →
Inovet © (2015-05-19 17:01) [35]> [22] Inovet © (19.05.15 14:18)
> Второй вкусней.
Но, блин, предпочитаю третий, когда строка с подстановками и функция Формат. Да, хуже, но универсальнее, да и строку в исходнике можно оформить в более адекватном виде.
← →
RWolf © (2015-05-19 17:38) [36]
> Игорь Шевченко © (19.05.15 12:16) [11]
> Пусть почитает на досуге:http://edn.embarcadero.com/article/10280
Почему все разработчики должны следовать стилю, принятому в своё время в Борланд?
← →
pavia © (2015-05-19 17:54) [37]Потому что Игорь Шевченко всегда прав. ☺
← →
Сергей Суровцев © (2015-05-19 18:12) [38]>RWolf © (19.05.15 17:38) [36]
>Почему все разработчики должны следовать стилю, принятому в своё время в Борланд?
Это не стиль Борланд. Это наиболее понятный и читаемый стиль как таковой. Именно поэтому его и использовали в Борланд. И другим советуют.
← →
han_malign © (2015-05-20 16:22) [39]
> Это не стиль Борланд.
>> http://edn.embarcadero.com/article/10280
>>> Delphi is created in California, so we discourage the use of notation ...
- а мне венгры таки ближе...
← →
Кто б сомневался © (2015-05-21 00:45) [40]Первый, но измененный.
Я бы весь текст вынес в отдельную строку константу и использовал 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]);
Но если код очень часто вызывается, то второй.
← →
Кто б сомневался © (2015-05-21 00:55) [41]
> его проще менять в будущем - напр. обычным юзерам
Если текст грузится с текстового файла.
← →
sniknik © (2015-05-21 10:36) [42]> Если текст грузится с текстового файла.
В [0] видно, что это код.
Если бы грузился юзерский файл, то, ИМХО, лучше подошёл бы вариант с парсером "аля PhP" предложенный выше, так чтобы переменные явно в текст записывались. Но вообще это офтопик. В обсуждении исключительно код + программист, и никаких юзеров.
← →
Дмитрий Белькевич © (2015-05-21 11:41) [43]+ за [40], типа такого что-то:
FormatStr :=
"ID перевода: %s" + CRLF + CRLF +
"Номер перевода: %s" + CRLF +
"Код точки: %s" + CRLF +
"Дата: %s" + CRLF + CRLF +
"Номер претензии: %s"+ CRLF +
"Комментарий: %s" + CRLF +
"Имя клиента: %s" + CRLF +
"Телефон: %s" + CRLF +
"Почта: %s";
sBody := Format(FormatStr, [aID, aTRN, ...]);
Номер претензии обрабатывать отдельно, до или после присвоения FormatStr.
← →
Дмитрий Белькевич © (2015-05-21 11:46) [44]Или const вместо FormatStr, да. Ничего же не меняется. ИМХО - наилучший вариант, но - тебе решать, конечно. Если нужна максимальная производительность - лучше вообще и без конкатенации и формата делать - копированием в длинную строку, с указателем на текущую вершину, мне кажется. Но, понятно, наглядность и прозрачность сильно пострадают. Названия переменных, опять же - плохие.
← →
sniknik © (2015-05-21 12:09) [45]> но - тебе решать, конечно
?
Как? На что повлияет "решение"? (или решил, что я спрашиваю для себя, свой стиль переформировать?)
Обычно я сталкиваюсь с таким постфактум, когда уже поздно "воспитывать" (это если для других) но нужно разбираться с проблемой/глюком (к счастью не в этом случае).
И обычно чужой код очень трудно понять... нелогичный, запутанный, будто специально куча усложнений.
Ну вот "твой"+[40] вариант, думаешь понятнее для чтения из-за "наведенного косметического сахара"?
Сталкивался. + То же самое у него, вот писал
> запросы вынесены в константы (+ поэтапное внесение... т.е. константу в переменную, а переменную после в CommandText)
Запрос в одном месте, использование в другом, а то и по разным модулям разнесено, параметры передаются через какие-то обезличенные наборы, и т.д.
Хотя, вроде бы просто, напиши прям там где используешь, и не нужно "мотаться" из модуля в модуль чтобы просто посмотреть как, и из чего сформируется запрос/письмо.
Вот сам прикинь, поставь себя на место читающего... в твоем случае. Вот тебе заявка "там с письмом что-то не то, данные не на месте вроде". Ну вот и проверь правильно ли у тебя соотносятся порядковые %s с какой нибудь переменной, не перепутан ли порядок... будет их 50, и что? Без переноса функции в тестовую прогу и задание явных значений (на месте проверить в 90% случаев нельзя/сложнее) получится? Просто прочитав код, дашь гарантию?
Хотя, ладно, чего я вообще убеждать начал... просто + еще пара человек чей код я бы не хотел получить на проверку.
← →
Дмитрий Белькевич © (2015-05-21 17:58) [46]>Просто прочитав код, дашь гарантию?
В [0] такая задача (над-модульный рефакторинг + проверки "на вшивость") не ставилась, как я вижу и понимаю, всё читать и плотно вникать в чужие проблемы, извини, нет времени. Для того, что написано в [0] локального случая, мне кажется, нормальное решение.
Я так бы вообще не делал, а делал бы шаблоны в файлах с названиями полей + парсер + данные в базе, у себя есть несколько немного похожих мест. Мне кажется - тянуть эту всю муть в код вообще не стоит, хотя, как я написал, дело твоё - ты задачу лучше знаешь и видишь.
← →
virex(home) © (2015-05-24 08:10) [47]свой формат с блекждеком
из стрингреплейс
[var1] и [var2] вместо %s в массиве параметров
а еще лучше: вместо массива искать свойство объекта из рантайма по имени
tmyobj = class
var1:string;
var2:string;
end;
myobj:=tmyobj.create...
myformat("text [var1] [var2]", myobj)
function myformat(s:string; obj:tobject)...
begin
... вытаскиваем из obj свойства и stringreplace на их содержимое
вот такой аналог синтакс сахара из си шарпа
← →
virex(home) © (2015-05-24 20:30) [48]в D7 можно так:
//{$M+} - включает rtti для TObject
//TMyObj можно наследовать и от TComponent, тогда модификатор не нужен
{$M+}
type TMyObj = class
private
Fvar1,Fvar2:string;
published
property var1:string read Fvar1 write Fvar1;
property var2:string read Fvar2 write Fvar2;
end;
{$M-}
function myformat(text:string; obj:TObject):string;
var
i,propCount:integer;
propInfo: PPropInfo;
propType: PPTypeInfo;
propList: PPropList;
begin
result:=text;
PropList := AllocMem(SizeOf(PropList^));
try
GetPropInfos(obj.ClassInfo, propList);
propCount := GetPropList(obj.ClassInfo, propList);
for i := 0 to propCount - 1 do
begin
propInfo := propList^[i];
propType := propInfo^.PropType;
case propType^.Kind of
tkString, tkLString, tkWString:
result:=StringReplace(result,"["+propInfo^.Name+"]",GetStrProp(obj, propInfo),[rfReplaceAll, rfIgnoreCase]);
end;
end;
finally
FreeMem(PropList);
end;
end;
procedure TForm1.Button1Click(Sender: TObject);
var obj:TMyObj;
begin
obj:=TMyObj.Create;
obj.var1:="hello";
obj.var2:="world";
Memo1.Text:=myformat("text: [var1] [var2]",obj);
end;
в Delphi 2010 и выше можно подключить rtti (псевдокод):function myformat(text:string; obj:TObject):string;
var ctx : TRttiContext;
rt : TRttiType;
prop : TRttiProperty;
value : TValue;
begin
result:=text;
ctx := TRttiContext.Create();
try
rt := ctx.GetType(obj.ClassType);
for prop in rt.GetProperties() do begin
case prop.PropertyType.TypeKind of
tkString, tkLString, tkWString, tkUString:
result:=StringReplace(result,"["+propInfo^.Name+"]",GetStrProp(obj, propInfo),[rfReplaceAll, rfIgnoreCase]);
else continue;
end;
end;
finally
ctx.Free();
end;
end;
← →
virex(home) © (2015-05-24 20:40) [49]
> virex(home) © (24.05.15 20:30) [48]
GetPropInfos тут лишнее конечно
и во втором варианте propInfo без ^
← →
Игорь Шевченко © (2015-05-25 11:04) [50]"Я нашел, как применить здесь
нестирающиеся шины из полиструктурного волокна с вырожденными аминными
связями и неполными кислородными группами. Но я не знаю пока, как
использовать регенерирующий реактор на субтепловых нейтронах. Миша, Мишок!
Как быть с реактором?"
← →
virex(home) © (2015-05-25 18:43) [51]думаю при 50+ переменных которые нужно вывести в удобочитаемый вид в качестве отчета, лучше напрячь rtti чем лепить конкатенацию на каждую переменную
← →
virex(home) © (2015-05-25 18:55) [52]А еще проще: сделать отдельный класс для отчетности
Добавлять переменную в виде пары ключ-значение
report.add("mail","office@mail.com")
report.add("call","8800-000-000")
...
report.print("почта: [mail], телефон: [call]")
← →
mike-d © (2015-05-25 19:13) [53]to Игорь Шевченко © (25.05.15 11:04) [50]
"Присмотревшись к устройству, я без труда узнал велосипед."
:)
Страницы: 1 2 вся ветка
Форум: "Прочее";
Текущий архив: 2016.01.24;
Скачать: [xml.tar.bz2];
Память: 0.62 MB
Время: 0.003 c