Объясните динозавру - C/C++

Почему это хорошо

BOOST_FOREACH(auto& file_name, files)
{
    if(boost::algorithm::ends_with(file_name, L"dll"))
    {
        ...
    }
}

а это

for (auto i = files.begin(); i != files.end(); i++)
{
    if (i->compare(i->size() - 3, 3, L"dll") == 0)
    {
        ...
    }
}

"ржу не могу всем отделом LOL" ???

Здравствуйте, Аноним, Вы писали:

А>Почему это хорошо

Разве не очевидно?

А>

А>BOOST_FOREACH(auto& file_name, files)
А>for (auto i = files.begin(); i != files.end(); i++)
А>

Первый вариант намного проще + читающий сразу понимает, что в теле foreach будет просмотрено каждое значение, без скачков.
Второй вариант — более error-prone, плюс в теле возможны скачки типа i-=10.

А>

А>if(boost::algorithm::ends_with(file_name, L"dll"))
А>if (i->compare(i->size() - 3, 3, L"dll") == 0)
А>

В первом варианте нет ничего лишнего — два значения и вызов предиката — проще и быть не может (разве что специальный кроссплатформенный предикат is_dynamic_library_name, но это уже другая логика).
Во втором же варианте добавляются вычисления с жёстко прибитыми длинами, опять же более error prone.

А>"ржу не могу всем отделом LOL" ???

А это скорее показатель культуры отдела.

Ну в общем особо то не важно, но если четко брать 2ой вариант, то проблема магические числа:

а если разрешение будете искать не dll а html, а тот кто залезет в код забудет тройки поменять?
Что будет если i->size() — 3 это выражение меньше нуля?

В первом коде таких проблем нет.

Здравствуйте, Аноним, Вы писали:

А>

А>for (auto i = files.begin(); i != files.end(); i++)
А>{
А>    if (i->compare(i->size() - 3, 3, L"dll") == 0)
А>    {
А>        ...
А>    }
А>}
А>

А>"ржу не могу всем отделом LOL" ???

Второй вариант оформления цикла мне нравится больше (что может понравиться в нестандартном макросе?).
А вот во втором варианте в условии, скорее всего, ошибка. Если имя файла короче трех символов мы будем иметь неопределенное поведение.

Вот это вот: "i", "size()", "3 и Lлитерал" — не тривиально и глаз зацепляется. У некоторых еще за "i++" будет зацепляться.

"i" заменить на file_name_it
size на length (они эквиваленты)

3 и Lлитерал на const std::wstring ext = L"dll"; ... // ext.length() вместо 3

Тогда будет не так топорно. Буст не обязателен.

Здравствуйте, Аноним, Вы писали:

Первый вариант легко починить, заменив условие на

boost::algorithm::iends_with(file_name, L".dll")

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

К уже сказанному выше.

Цикл первого варианта уже можно заменить на стандартную конструкцию
for (auto& file_name : files)

> for (auto i = files.begin(); i != files.end(); i++)
end() вычисляется каждый раз, на практике эта операция не может быть закеширована в регистр или стек.
в случае первого варианта end() вычисляется оидн раз.

Здравствуйте, Аноним, Вы писали:

А>Почему это хорошо

А>

А>BOOST_FOREACH(auto& file_name, files)
А>{
А>    if(boost::algorithm::ends_with(file_name, L"dll"))
А>    {
А>        ...
А>    }
А>}
А>

А>а это

А>

А>for (auto i = files.begin(); i != files.end(); i++)
А>{
А>    if (i->compare(i->size() - 3, 3, L"dll") == 0)
А>    {
А>        ...
А>    }
А>}
А>

А>"ржу не могу всем отделом LOL" ???

Раз есть auto, значит должна быть и лямбда, чем плох такой вариант?:

std::for_each(files.begin(), files.end(), [](const file &)
{
...
});

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

_>

_>std::for_each(files.begin(), files.end(), [](const file &)
_>{
_>...
_>});
_>

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

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

А>>"ржу не могу всем отделом LOL" ???
_>Раз есть auto, значит должна быть и лямбда, чем плох такой вариант?:

_>

_>std::for_each(files.begin(), files.end(), [](const file &)
_>{
_>...
_>});
_>

Только отсутствием полиморфной лямбды (если же выписывать decltype — то довольно неуклюже будет выглядеть). (+лучше тогда уже boost::for_each)

Здравствуйте, Аноним, Вы писали:

А>Почему это хорошо
А>

А>BOOST_FOREACH(auto& file_name, files)
А>

Это плохо использованием макроса

А>а это
А>

А>for (auto i = files.begin(); i != files.end(); i++)
А>

можно соптимизировать

А>

А>    if (i->compare(i->size() - 3, 3, L"dll") == 0)
А>

UB, как уже было указано.

А>"ржу не могу всем отделом LOL" ???
Действительно. Оба примера содержат ошибку. Попробуйте такие имена как "ItIsNotdll", "dll".

Здравствуйте, Son of Northern Darkness, Вы писали:

SON>Обычно такое не удобно отлаживать. В каждой итерации приходится проваливаться в код std, да и читаемость ниже.

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

Здравствуйте, Evgeny.Panasyuk, Вы писали:

EP>Про отладку ещё можно понять, но с читаемостью-то что?
EP>Наоборот, ведь, выше — сразу видно что код выполнится для каждого элемента.

Да подсветка, например.

Здравствуйте, Son of Northern Darkness, Вы писали:

EP>>Про отладку ещё можно понять, но с читаемостью-то что?
EP>>Наоборот, ведь, выше — сразу видно что код выполнится для каждого элемента.
SON>Да подсветка, например.

А что с подсветкой?

Здравствуйте, Evgeny.Panasyuk, Вы писали:

EP>Здравствуйте, Son of Northern Darkness, Вы писали:

EP>>>Про отладку ещё можно понять, но с читаемостью-то что?
EP>>>Наоборот, ведь, выше — сразу видно что код выполнится для каждого элемента.
SON>>Да подсветка, например.

EP>А что с подсветкой?

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

Здравствуйте, Alexander G, Вы писали:

AG>end() вычисляется каждый раз, на практике эта операция не может быть закеширована в регистр или стек.
AG>в случае первого варианта end() вычисляется оидн раз.
не совсем так.
компилятор смотрит, если по отношению к 'files' применяются только константные методы, то 'files.end()' вычисляется только один раз. (говорю про GCC)

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

AG>>end() вычисляется каждый раз, на практике эта операция не может быть закеширована в регистр или стек.
AG>>в случае первого варианта end() вычисляется оидн раз.
X>не совсем так.
X>компилятор смотрит, если по отношению к 'files' применяются только константные методы, то 'files.end()' вычисляется только один раз. (говорю про GCC)

Не может такого быть, из-за mutable или хотя бы указателей. Компилятору нужно доказать что результат не меняется.

Здравствуйте, Evgeny.Panasyuk, Вы писали:

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

AG>>>end() вычисляется каждый раз, на практике эта операция не может быть закеширована в регистр или стек.
AG>>>в случае первого варианта end() вычисляется оидн раз.
X>>не совсем так.
X>>компилятор смотрит, если по отношению к 'files' применяются только константные методы, то 'files.end()' вычисляется только один раз. (говорю про GCC)

EP>Не может такого быть, из-за mutable или хотя бы указателей. Компилятору нужно доказать что результат не меняется.

Вполне может иметь место хак применительно к стандартным контейнерам

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

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

EP>>Не может такого быть, из-за mutable или хотя бы указателей. Компилятору нужно доказать что результат не меняется.
_>Вполне может иметь место хак применительно к стандартным контейнерам

Это требует кооперации между GCC и библиотекой, скорей всего каких-то атрибутов. А вот есть ли такие атрибуты?

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

AFAIK, не immutable, а thread-safe, что более слабая гарантия.

Здравствуйте, Evgeny.Panasyuk, Вы писали:

EP>Не может такого быть, из-за mutable или хотя бы указателей. Компилятору нужно доказать что результат не меняется.
просто проверь.

А>Почему это хорошо
А>

А>BOOST_FOREACH(auto& file_name, files)
А>

А>а это
А>

А>for (auto i = files.begin(); i != files.end(); i++)
А>

Изучение первого варианта требует дополнительного времени и возможно только после изучения второго. Значит программист может заработать больше, поскольку это знание позволяет написать в резюме не просто "C++/STL", а уже "C++/STL/boost".

А>"ржу не могу всем отделом LOL" ???

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

EP>>Не может такого быть, из-за mutable или хотя бы указателей. Компилятору нужно доказать что результат не меняется.
X>просто проверь.

То что он сможет доказать, особенно после inline'а, и "вызывать" только один раз — я не сомневаюсь.
Вопрос в том как именно он это докажет. Я думаю вряд ли только по const на методах — для этого ещё потребуются атрибуты, так как const никак не достаточно.
А вот после inline'инга, да ещё и в SSA — без проблем.

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

А>>Почему это хорошо
А>>

А>>BOOST_FOREACH(auto& file_name, files)
А>>

А>>а это
А>>

А>>for (auto i = files.begin(); i != files.end(); i++)
А>>

MQ>Изучение первого варианта требует дополнительного времени и возможно только после изучения второго.

Неверно. Можно пользоваться первым вариантом, даже не зная второй.
Мой любимый пример — книга Страуструпа Programming -- Principles and Practice Using C++ — std::vector вводится где-то на сотой странице, а new только где-то в середине, странице на пятисотой.

Здравствуйте, Alexander G, Вы писали:

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

Если заморачиваться такими мелкими не-оптимальностями, то стоит также отметить, что boost::algorithm::ends_with из первого варианта будет на каждой итерации считать длину параметра-литерала. И если ставить цель "ни такта в пустую", то хорошо бы туда передавать размер, уже известный на этапе компиляции, с помощью какого-нибудь варианта string_view.

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

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

R>Если заморачиваться такими мелкими не-оптимальностями, то стоит также отметить, что boost::algorithm::ends_with из первого варианта будет на каждой итерации считать длину параметра-литерала.

Длина литерала известна в compile time, ends_with — это шаблон, то есть принимает range'ы без потерь этой информации. Если там как-то внутри не сильно накрутили — у компилятора есть вся информация чтобы не вычислять длину, а использоваться константу.

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

R>Если заморачиваться такими мелкими не-оптимальностями
Оптимизациями не имеет смысл заморачиваться не потому, что тама что-то сколько-то раз вызывается внутре, а потому, что мы ничего не знаем о типах. Может контейнер files работает по сети, телеграфу, ямщиками...

Здравствуйте, Аноним, Вы писали:

А>Почему это хорошо

Это не хорошо. Ручная проверка расширения файла.
Если нет упора на высокую производительность, то лучше посмотреть в сторону boost::filesystem::path, а если есть — написать корректную функцию проверки расширения.
(Нынешняя реализация проглатывает "helloworldll" и "thisis.notdll")

А>а это
А>"ржу не могу всем отделом LOL" ???

Если есть auto, то должен быть и новый синтаксис

for(const auto& file_name : files)

Да и внутри уровень велосипеделия ещё выше.

Здравствуйте, Кодт, Вы писали:

К>Здравствуйте, Аноним, Вы писали:

А>>Почему это хорошо

К>Это не хорошо. Ручная проверка расширения файла.
К>Если нет упора на высокую производительность, то лучше посмотреть в сторону boost::filesystem::path, а если есть — написать корректную функцию проверки расширения.
К>(Нынешняя реализация проглатывает "helloworldll" и "thisis.notdll")

А>>а это
А>>"ржу не могу всем отделом LOL" ???

К>Если есть auto, то должен быть и новый синтаксис
К>

К>for(const auto& file_name : files)
К>

К>Да и внутри уровень велосипеделия ещё выше.

не совсем, 10 студия этого не умеет

Здравствуйте, Кодт, Вы писали:

К>Здравствуйте, Аноним, Вы писали:

А>>Почему это хорошо

К>Это не хорошо. Ручная проверка расширения файла.
К>Если нет упора на высокую производительность, то лучше посмотреть в сторону boost::filesystem::path, а если есть — написать корректную функцию проверки расширения.
К>(Нынешняя реализация проглатывает "helloworldll" и "thisis.notdll")

Не пойму, чего все привязались к проверке расширения

Вообще же топик не о том, в нём о расширениях файлов вообще ни слова.

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

T>Не пойму, чего все привязались к проверке расширения

Вообще же топик не о том, в нём о расширениях файлов вообще ни слова.

А о чём? В одном случае два раза буст, в другом случае два раза рукоделие.
И таки-да, в топике о расширениях файлов вообще слова есть.
Особенно убивает i->size()-3 — попробуй ему скормить строку "xz", узнаешь много новых слов. Говнокод детектед, неудивительно, что коллеги ржут.

Здравствуйте, Аноним, Вы писали:

А>Почему это хорошо

А>[ccode]
А>BOOST_FOREACH(auto& file_name, files)

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

Здравствуйте, Кодт, Вы писали:

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

T>>Не пойму, чего все привязались к проверке расширения

Вообще же топик не о том, в нём о расширениях файлов вообще ни слова.

К>А о чём? В одном случае два раза буст, в другом случае два раза рукоделие.
Я думаю, что топик был о том, зачем тащить в код всякие "новомодные" фичи, без которых 100 лет все жили.

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

К>Особенно убивает i->size()-3 — попробуй ему скормить строку "xz", узнаешь много новых слов. Говнокод детектед, неудивительно, что коллеги ржут.
А это просто демонстрация принципа "чем меньше кода, тем меньше ошибок", и того, что да, бездумно используя STL (да и вообще что угодно) можно оказаться в самых неожиданных положениях.

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

К>>А о чём? В одном случае два раза буст, в другом случае два раза рукоделие.
T>Я думаю, что топик был о том, зачем тащить в код всякие "новомодные" фичи, без которых 100 лет все жили.

Пусть тогда Керниган и Ричи залогинится.

К>>И таки-да, в топике о расширениях файлов вообще слова есть.
T>Честно говоря, в сообщении автора я что-то не нахожу этих слов, может куда-то не туда смотрю.

file_name, ends_with, "dll" — самодокументируемый код

Здравствуйте, Son of Northern Darkness, Вы писали:

SON>Второй вариант оформления цикла мне нравится больше (что может понравиться в нестандартном макросе?).

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

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

_>не совсем, 10 студия этого не умеет

Умеет, но со своим нестандартным синтаксисом — досталось из поддержки CLI, но работает и в чисто нативном коде.

http://msdn.microsoft.com/en-us/library/ms177202%28v=vs.100%29.aspx

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

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

_>>не совсем, 10 студия этого не умеет

Ops>Умеет, но со своим нестандартным синтаксисом — досталось из поддержки CLI, но работает и в чисто нативном коде.

Ops>http://msdn.microsoft.com/en-us/library/ms177202%28v=vs.100%29.aspx

За такое в приличном обществе по рукам бьют

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

_>За такое в приличном обществе по рукам бьют

Мопед не мой, сам ни разу такое не писал

К>>А о чём? В одном случае два раза буст, в другом случае два раза рукоделие.
T>Я думаю, что топик был о том, зачем тащить в код всякие "новомодные" фичи, без которых 100 лет все жили.
Новомодные фичи в данном конкретном случае помогают избавиться от части багов в коде (имяфайла короче 3-х символов).

К>>И таки-да, в топике о расширениях файлов вообще слова есть.
T>Честно говоря, в сообщении автора я что-то не нахожу этих слов, может куда-то не туда смотрю.
Да,да. Автор хотел отфильтровать в том числе и такие файлы "adll", "b.cdll".

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

Ops>>Умеет, но со своим нестандартным синтаксисом — досталось из поддержки CLI, но работает и в чисто нативном коде.
Ops>>http://msdn.microsoft.com/en-us/library/ms177202%28v=vs.100%29.aspx
_>За такое в приличном обществе по рукам бьют

Не по рукам, а канделябром по голове

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

К>>

К>>for(const auto& file_name : files)
К>>

_>не совсем, 10 студия этого не умеет

13 вполне умеет.

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

К>>>

К>>>for(const auto& file_name : files)
К>>>

_>>не совсем, 10 студия этого не умеет

bnk>13 вполне умеет.

Умеют 2012 и 2013.

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

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

__>я за второй вариант, так как, как минимум, первый требует буста, да и синтаксически коряв

Приноси канделябр с собой

Здравствуйте, Кодт, Вы писали:

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

__>>я за второй вариант, так как, как минимум, первый требует буста, да и синтаксически коряв

К>Приноси канделябр с собой

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

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

__>(по-моему она появилась уже в каком-то стандарте, если не ошибаюсь?)

Появился в C++11, называется range-based for. Вообще, при наличии полиморфных лямбд (C++14), он не сильно и нужен:

for(auto x : expr)
{
    use(x);
}
// versus
for_each(expr, [](auto x)
{
    use(x);
});

Вариант с for_each даже более удобен, так как время жизни всех временных объектов в expr более естественное, а в range-based for можно получить сюрпризы.
С другой стороны, появляются вопросы про break, continue, return и т.п.

	От:	Аноним
	Дата:	11.04.14 07:02
	Оценка:

	От:	Evgeny.Panasyuk
	Дата:	11.04.14 07:19
	Оценка:	1 (1) +1

	От:	Trrrrr
	Дата:	11.04.14 07:19
	Оценка:	1 (1) +2

	От:	Son of Northern Darkness
	Дата:	11.04.14 07:23
	Оценка:	5 (2) +3

	От:	oziro
	Дата:	11.04.14 07:28
	Оценка:

От:	niXman	https://github.com/niXman
Дата:	11.04.14 08:44
Оценка:	1 (1)

От:	bnk	http://unmanagedvisio.com/
Дата:	11.04.14 17:23
Оценка: