Интерфейсы для строковых классов обычно имеют метод с именем IsEmpty
(VCL) или же empty
(СТЛ). Это абсолютно разумно, потому что это особый случай, но код, который использует эти методы, часто должен опровергать этот предикат, что приводит к «оптический (и даже психологический) накладные расходы» (восклицательный знак не очень очевиден, особенно после открывающей скобки). Смотрите, например, этот (упрощенный) код:
/// format an optional time specification for output
std::string fmtTime(const std::string& start, const std::string& end)
{
std::string time;
if (!start.empty() || !end.empty()) {
if (!start.empty() && !end.empty()) {
time = "from "+start+" to "+end;
} else {
if (end.empty()) {
time = "since "+start;
} else {
time = "until "+end;
}
}
}
return time;
}
Она имеет четыре отрицания, потому что пустые ящики пропускаются. Я часто наблюдаю такого рода отрицание, также при разработке интерфейсов, и это не большая проблема но это раздражает. Я только хочу поддержать написание понятного и легкого для чтения кода. Надеюсь, вы поймете мою мысль.
Может быть, меня поразила только слепота: как бы вы решили вышеупомянутую проблему?
Редактировать: Прочитав некоторые комментарии, я думаю, что необходимо сказать, что исходный код использует класс System::AnsiString
VCL. Этот класс обеспечивает IsEmpty
метод, который очень читабелен:
if (text.IsEmpty()) { /* ... */ } // read: if text is empty ...
если не отрицается:
if (!text.IsEmpty()) { /* ... */} // read: if not text is empty ...
…вместо если текст не пустой. Я думаю буквальный is
Лучше оставить фантазию читателя, чтобы и отрицание работало хорошо. Хорошо, возможно не распространенная проблема …
В большинстве случаев вы можете изменить порядок if
и else
очистить код:
const std::string fmtTime(const std::string& start, const std::string& end)
{
std::string time;
if (start.empty() && end.empty()) {
return time;
}
if (start.empty() || end.empty()) {
if (end.empty()) {
time = "since "+start;
} else {
time = "until "+end;
}
} else {
time = "from "+start+" to "+end;
}
return time;
}
Или даже чище после некоторого рефакторинга:
std::string fmtTime(const std::string& start, const std::string& end)
{
if (start.empty() && end.empty()) {
return std::string();
}
if (start.empty()) {
return "until "+end;
}
if (end.empty()) {
return "since "+start;
}
return "from "+start+" to "+end;
}
И для максимальной компактности (хотя я предпочитаю предыдущую версию, из-за ее читабельности):
std::string fmtTime(const std::string& start, const std::string& end)
{
return start.empty() && end.empty() ? std::string()
: start.empty() ? "until "+end
: end.empty() ? "since "+start
: "from "+start+" to "+end;
}
Другая возможность — создать вспомогательную функцию:
inline bool non_empty(const std::string &str) {
return !str.empty();
}
if (non_empty(start) || non_empty(end)) {
...
}
Я думаю, что я устраню условия в пользу небольшой математики:
const std::string fmtTime(const std::string& start, const std::string& end) {
typedef std::string const &s;
static const std::function<std::string(s, s)> f[] = {
[](s a, s b) { return "from " + a + " to " + b; }
[](s a, s b) { return "since " + a; },
[](s a, s b) { return "until " + b; },
[](s a, s b) { return ""; },
};
return f[start.empty() * 2 + end.empty()](start, end);
}
Изменить: если вы предпочитаете, вы можете выразить математику как start.empty() * 2 + end.empty()
, Чтобы понять, что происходит, возможно, будет лучше, если я объясню, как я думал о том, с чего начать. Я думал о вещах как о двухмерном массиве:
(Не стесняйтесь поменять местами «начало пусто» и «конец пусто», в зависимости от того, предпочитаете ли вы думать в мажоре строк или столбцов).
start.empty()
а также end.empty()
(или логический not
из них, если вы предпочитаете) каждый действует как индекс вдоль одного измерения этой 2D матрицы. Математическая процедура просто «линеаризует» эту адресацию, поэтому вместо двух строк и двух столбцов мы получаем одну длинную строку, что-то вроде этого:
С математической точки зрения, это просто вопрос «строка * столбцы + столбец» (или, наоборот, в зависимости от того, предпочитаете ли вы порядок строк — основной или столбец). Я изначально выразил * 2
часть как бит-сдвиг и сложение как побитовый or
(зная, что младший значащий бит пуст, из-за предыдущего сдвига влево). Мне легко с этим справиться, но я думаю, что могу понять, где другие не могут.
Я, вероятно, должен добавить: хотя я уже упоминал мажор строки по сравнению с мажором столбца, должно быть совершенно очевидно, что отображение двух значений «x.empty» на позиции в массиве в основном произвольно. Значение, которое мы получаем от .empty()
означает, что мы получаем 0, когда значение отсутствует, и 1, когда оно есть. Таким образом, прямое отображение исходных значений в позиции массива, вероятно, выглядит следующим образом:
Поскольку мы линеаризуем значение, у нас есть несколько вариантов того, как мы делаем отображение:
!x.empty()
)Для тех, кто сомневается в эффективности этого, он фактически сводится к этому (с VC ++):
mov eax, ebx
cmp QWORD PTR [rsi+16], rax
sete al
cmp QWORD PTR [rdi+16], 0
sete bl
lea eax, DWORD PTR [rbx+rax*2]
movsxd rcx, eax
shl rcx, 5
add rcx, r14
mov r9, rdi
mov r8, rsi
mov rdx, rbp
call <ridiculously long name>::operator()
Даже одноразовая конструкция для f
не так плохо, как некоторые могут подумать. Это не включает динамическое распределение или что-то в этом порядке. Имена достаточно длинные, что поначалу выглядит немного страшно, но, в конце концов, это в основном четыре повторения:
lea rax, OFFSET FLAT:??_7?$_Func_impl@U?$_Callable_obj@V<lambda_f466b26476f0b59760fb8bb0cc43dfaf>@@$0A@@std@@V?$allocator@V?$_Func_class@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@AEBV12@AEBV12@@std@@@2@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@2@AEBV42@AEBV42@@std@@6B@
mov QWORD PTR f$[rsp], rax
Оставляя вне static const
похоже, не сильно влияет на скорость выполнения. Поскольку таблица является статической, я думаю, что она должна быть там, но что касается скорости выполнения, это не тот массовый выигрыш, который мы могли бы ожидать, если бы инициализация таблицы включала четыре отдельных динамических распределения или что-то в этом роде.
Ты мог бы сказать
if (theString.size()) { .... }
Является ли это более читабельным, другой вопрос. Здесь вы вызываете метод, основная цель которого не состоит в том, чтобы сообщать вам, если вещь пуста, и полагаетесь на неявное преобразование в bool
, Я бы предпочел !s.empty()
версия. Я мог бы использовать not
вместо забавы:
if (not theString.empty()) { .... }
Было бы интересно увидеть корреляцию между людьми, которые находят !
а также not
версии сбивают с толку.
Я должен рефакторинг этого, чисто из-за анального ретенционного расстройства …
std::string fmtTime( const std::string & start, const std::string & end ) {
if ( start.empty() ) {
if ( end.empty() ) return ""; // should diagnose an error here?
return "until " + end;
}
if ( end.empty() ) return "since " + start;
return "from " + start + " to " + end;
}
Там… чисто, чисто, чисто. Если что-то здесь трудно прочитать, добавьте комментарий, а не другой if
пункт.
Обычно лучше не использовать такой сложный условный код. Почему бы не сделать это простым?
const std::string fmtTime(const std::string& start, const std::string& end)
{
if (start.empty() && end.empty())
{
return "";
}
// either start or end or both are not empty here.
std::string time;
if (start.empty())
{
time = "until "+end;
}
else if (end.empty())
{
time = "since "+start;
}
else // both are not empty
{
time = "from "+start+" to "+end;
}
return time;
}
Во всем мире у меня нет проблем с тем, как ты это написал; его
конечно чище, что альтернативы, которые другие
предлагая. Если вы беспокоитесь о !
исчезающий (который
это законное беспокойство), используйте больше пробелов.
if ( ! start.empty() || ! end.empty() ) ...
Или попробуйте использовать ключевое слово not
вместо:
if ( not start.empty() || not end.empty() ) ...
(С большинством редакторов not
будет выделено как ключевое слово,
что привлечет к нему еще больше внимания.)
В противном случае две вспомогательные функции:
template <typename Container>
bool
isEmpty( Container const& container )
{
return container.empty();
}
template <typename Container>
bool
isNotEmpty( Container const& container )
{
return !container.empty();
}
Это имеет дополнительное преимущество предоставления функциональности
лучшее имя. (Названия функций являются глаголами, поэтому c.empty()
логически означает «опустошить контейнер», а не «является контейнером»
empty «. Но если вы начнете упаковывать все функции в
стандартная библиотека с плохими именами
для вас.)
Без использования отрицания ..;)
const std::string fmtTime(const std::string& start, const std::string& end)
{
std::string ret;
if (start.empty() == end.empty())
{
ret = (start.empty()) ? "" : "from "+start+" to "+end;
}
else
{
ret = (start.empty()) ? "until "+end : "since "+start;
}
return ret;
}
РЕДАКТИРОВАТЬ: ладно убрал немного больше …
Так как никто не хотел напечатать полный ответ с моим комментарием, здесь это идет:
Создайте локальные переменные, которые упрощают чтение выражений:
std::string fmtTime(const std::string& start, const std::string& end)
{
std::string time;
const bool hasStart = !start.empty();
const bool hasEnd = !end.empty();
if (hasStart || hasEnd) {
if (hasStart && hasEnd) {
time = "from "+start+" to "+end;
} else {
if (hasStart) {
time = "since "+start;
} else {
time = "until "+end;
}
}
}
return time;
}
Компилятор достаточно умен, чтобы исключить эти переменные, и даже если этого не произойдет, он не будет менее эффективным, чем оригинал (я ожидаю, что оба будут одним тестом переменной). Код сейчас немного больше удобочитаемый для человека, который может просто прочитать условия:
если имеет начало или конец, то
Конечно, вы могли бы также сделать различные рефакторы, чтобы еще больше упростить число вложенных операций, таких как выделение, когда нет начала или конца, и выручка пораньше …