style guide google coding code c++ string api coding-style readability

c++ - guide - ¿Hay un contrario conciso de "está vacío"?



google code style java (11)

¿Considerarías asignado un buen opuesto?

#include <string> template <typename CharType> bool assigned(const std::basic_string<CharType>& s) { return !s.empty(); } std::string fmtTimeSpec(const std::string& from, const std::string& to) { if (assigned(from)) { if (assigned(to)) { return "from "+from+" to "+to; } return "since "+from; } if (assigned(to)) { return "until "+to; } return std::string(); }

Las mejoras estructurales de la "función de prueba" provienen de numerosas respuestas útiles. Agradecimientos especiales a:

  • CE Gesser
  • Potatoswatter

Las interfaces para las clases de cadenas suelen tener el método llamado IsEmpty ( VCL ) o empty ( STL ). Eso es absolutamente razonable porque es un caso especial, pero el código que usa estos métodos a menudo tiene que negar este predicado, lo que conduce a una " sobrecarga óptica (e incluso psicológica) " (el signo de exclamación no es muy obvio, especialmente después de un paréntesis de apertura ). Ver por ejemplo este código (simplificado):

/// 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; }

Tiene cuatro negaciones , porque los casos vacíos son los que deben omitirse. A menudo observo este tipo de negación, también al diseñar interfaces, y no es un gran problema, pero es molesto. Solo deseo apoyar la escritura de código comprensible y fácil de leer. Espero que entiendas mi punto.

Tal vez solo me sorprenda la ceguera: ¿Cómo resolvería el problema anterior?

Edit: Después de leer algunos comentarios, creo que es necesario decir que el código original utiliza la clase System::AnsiString de la VCL. Esta clase proporciona un método IsEmpty , que es muy legible:

if (text.IsEmpty()) { /* ... */ } // read: if text is empty ...

si no es negado

if (!text.IsEmpty()) { /* ... */} // read: if not text is empty ...

... en lugar de si el texto no está vacío . Creo que is mejor dejar el literal a la fantasía del lector para que también la negación funcione bien. Ok, tal vez no sea un problema generalizado ...


A nivel mundial, no tengo ningún problema con la forma en que lo has escrito; Es ciertamente más limpio que las alternativas que otros proponen. Si estás preocupado por el ! Desapareciendo (que es una preocupación legítima), usa más espacio en blanco.

if ( ! start.empty() || ! end.empty() ) ...

O intente usar la palabra clave not lugar:

if ( not start.empty() || not end.empty() ) ...

(Con la mayoría de los editores, el not se resaltará como una palabra clave, lo que atraerá aún más la atención).

De lo contrario, dos funciones auxiliares:

template <typename Container> bool isEmpty( Container const& container ) { return container.empty(); } template <typename Container> bool isNotEmpty( Container const& container ) { return !container.empty(); }

Esto tiene la ventaja adicional de dar un mejor nombre a la funcionalidad. (Los nombres de funciones son verbos, por lo que c.empty() significa lógicamente "vaciar el contenedor", y no "está vacío el contenedor". Pero si comienza a ajustar todas las funciones de la biblioteca estándar que tienen nombres incorrectos, consiguió su trabajo cortado para usted.)


Como a nadie le importó escribir la respuesta completa con mi comentario, aquí va:

Crea variables locales que simplifiquen la lectura de expresiones:

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; }

El compilador es lo suficientemente inteligente como para evitar esas variables, e incluso si no lo fuera, no será menos eficiente que el original (espero que ambas sean una sola prueba de una variable). El código ahora es un poco más legible para un humano que simplemente puede leer las condiciones:

si tiene comienzo o fin entonces

Por supuesto, también puede hacer refactores diferentes para simplificar aún más el número de operaciones anidadas, como seleccionar solo cuando no hay un inicio o un final y rescatarse temprano ...


Creo que eliminaría las condiciones a favor de un poco de matemáticas:

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); }

Edición: si lo prefiere, puede expresar las matemáticas como start.empty() * 2 + end.empty() . Para entender lo que está pasando, tal vez sea mejor si expongo cómo pensé en las cosas para empezar. Pensé en las cosas como una matriz 2D:

(No dude en intercambiar el "inicio vacío" y "finalizar vacío", dependiendo de si prefiere pensar en orden de fila mayor o de columna principal).

start.empty() y end.empty() (o la lógica not de ellos, si lo prefiere) actúan como un índice a lo largo de una dimensión de esta matriz 2D. Las matemáticas involucradas simplemente "linealizan" ese direccionamiento, así que en lugar de dos filas y dos columnas, obtenemos una fila larga, algo como esto

En términos matemáticos, eso es una cuestión simple de "filas * columnas + columnas" (o, nuevamente, viceversa, dependiendo de si prefiere la fila mayor o la columna mayor). Originalmente expresé la parte * 2 como un cambio de bits y la adición como un bit or sabiendo que el bit menos significativo está vacío, debido al desplazamiento anterior a la izquierda. Me resulta fácil tratar con eso, pero creo que puedo entender dónde otros no podrían.

Probablemente debería agregar: aunque ya he mencionado la fila principal frente a la columna principal, debería ser bastante obvio que la asignación de los dos valores "x.empty" a las posiciones en la matriz es básicamente arbitraria. El valor que obtenemos de .empty() significa que obtenemos un 0 cuando el valor no está presente, y un 1 cuando está. Como tal, una asignación directa de los valores originales a las posiciones de la matriz es probablemente así:

Ya que estamos linealizando el valor, tenemos algunas opciones para la forma en que hacemos el mapeo:

  1. simplemente organiza la matriz de acuerdo con los valores a medida que los obtenemos.
  2. invierta el valor para cada dimensión individualmente (esto es básicamente lo que condujo a la pregunta original: el uso constante de !x.empty() )
  3. Combine las dos entradas en una sola dirección lineal, luego "invierta" restando de 3.

Para aquellos que dudan de la eficiencia de esto, en realidad compila esto (con 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()

Incluso la construcción de una sola vez para f no es tan mala como algunos podrían pensar. No implica asignación dinámica, ni nada en ese orden. Los nombres son lo suficientemente largos para que parezca un poco aterrador al principio, pero al final, son casi cuatro repeticiones de:

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

Dejar la static const no parece afectar mucho la velocidad de ejecución. Dado que la tabla es estática, creo que debería estar allí, pero en lo que respecta a la velocidad de ejecución, no es el tipo de ganancia masiva que podríamos esperar si la inicialización de la tabla implicara cuatro asignaciones dinámicas separadas, o algo así.


En la mayoría de los casos, puede invertir el orden de if y the else para limpiar el código:

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; }

O incluso más limpio después de un poco más de refactorización:

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; }

Y para la máxima compacidad (aunque prefiero la versión anterior, por su legibilidad):

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; }

Otra posibilidad es crear una función auxiliar:

inline bool non_empty(const std::string &str) { return !str.empty(); } if (non_empty(start) || non_empty(end)) { ... }


Por lo general, es mejor no usar un código condicional tan complicado. ¿Por qué no mantenerlo simple?

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; }


Si todo lo que te preocupa es la facilidad con la que ! se puede pasar por alto, puede usar el token alternativo estándar de C ++ not lugar:

const std::string fmtTime(const std::string& start, const std::string& end) { std::string time; if (not start.empty() or not end.empty()) { if (not start.empty() and not end.empty()) { time = "from "+start+" to "+end; } else { if (end.empty()) { time = "since "+start; } else { time = "until "+end; } } } return time; }

(Consulte [lex.digraph] en el estándar para tokens alternativos)


Sin usar la negación ..;)

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; }

EDIT: bien limpiado un poco más ...


También lucho con la sobrecarga psicológica de la lógica negativa.

Una solución para esto (cuando no se puede evitar) es verificar la condición explícita, considerar:

if (!container.empty())

vs

if (container.empty() == false)

La segunda versión es más fácil de leer porque fluye como lo leerías en voz alta. También deja claro que está comprobando una condición falsa.

Ahora, si eso todavía no es lo suficientemente bueno para usted, mi consejo sería crear una clase de envoltura delgada que herede de cualquier contenedor que esté utilizando y luego crear su propio método para esa comprobación en particular.

Por ejemplo con cuerdas:

class MyString : public std::string { public: bool NotEmpty(void) { return (empty() == false); } };

Ahora se vuelve justo:

if (container.NotEmpty())...


Tengo que refactorizar esto, puramente por un desorden retentivo anal ...

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; }

Ahí ... limpio limpio limpio limpio Si es difícil leer algo aquí, agregue un comentario, no otra cláusula if .


Tu puedes decir

if (theString.size()) { .... }

Si eso es más legible es un asunto diferente. Aquí está llamando a un método cuyo propósito principal no es decirle si la cosa está vacía, y confiar en una conversión implícita a bool . Preferiría la !s.empty() . Podría usar not lugar de diversión:

if (not theString.empty()) { .... }

Podría ser interesante ver la correlación entre las personas que encuentran el ! Y not versiones confusas.