Separación de preocupaciones: ¿Cuándo es la separación "demasiado"?

Realmente amo el código limpio y siempre quiero codificar mi código de la mejor manera posible. Pero siempre había una cosa que realmente no entendía:

¿Cuándo es demasiado de "separación de preocupaciones" con respecto a los métodos?

Digamos que tenemos el siguiente método:

def get_last_appearance_of_keyword(file, keyword):
    with open(file, 'r') as file:
        line_number = 0
        for line in file:
            if keyword in line:
                line_number = line
        return line_number

Creo que este método está bien como está. Es simple, fácil de leer y claramente lo que dice su nombre. Pero: en realidad no está haciendo "solo una cosa". En realidad, abre el archivo y luego lo encuentra. Eso significa que podría dividirlo aún más (también teniendo en cuenta el "Principio de responsabilidad única"):

Variación B (Bueno, esto tiene sentido de alguna manera. De esta manera podemos reutilizar fácilmente el algoritmo de encontrar la última aparición de una palabra clave en un texto, sin embargo, parece "demasiado". No puedo explicar por qué, pero simplemente "siento" "de esa manera):

def get_last_appearance_of_keyword(file, keyword):
    with open(file, 'r') as text_from_file:
        line_number = find_last_appearance_of_keyword(text_from_file, keyword) 
    return line_number

def find_last_appearance_of_keyword(text, keyword):
    line_number = 0
    for line in text:
        if keyword in line:
            line_number = line
    return line_number

Variación C (Esto es simplemente absurdo en mi opinión. Básicamente estamos encapsulando una línea en otro método con solo una línea dos veces. Pero se podría argumentar que la forma de abrir algo puede cambiar en el futuro, debido a algunas solicitudes de características , y como no queremos cambiarlo muchas veces, pero solo una vez, simplemente lo encapsulamos y separamos nuestra función principal aún más):

def get_last_appearance_of_keyword(file, keyword):
    text_from_file = get_text_from_file(file)
    line_number = find_keyword_in_text(text_from_file, keyword)
    return line_number 

def get_text_from_file(file):
    with open(file, 'r') as text:
        return text

def find_last_appearance_of_keyword(text, keyword):
    line_number = 0
    for line in text:
        if check_if_keyword_in_string(line, keyword):
            line_number = line         
    return line_number

def check_if_keyword_in_string(text, keyword):
    if keyword in string:
        return true
    return false

Entonces mi pregunta ahora: ¿Cuál es la forma correcta de escribir este código y por qué los otros enfoques son correctos o incorrectos? Siempre aprendí: Separación, pero nunca cuando es simplemente demasiado. ¿Y cómo puedo estar seguro en el futuro, que es "justo" y que no necesita más separación cuando estoy codificando nuevamente?

Sus diversos ejemplos de división de preocupaciones en funciones separadas tienen el mismo problema: todavía está codificando la dependencia del archivo get_last_appearance_of_keyword. Esto hace que la función sea difícil de probar, ya que ahora tiene que responder en un archivo existente en el sistema de archivos cuando se ejecuta la prueba. Esto lleva a pruebas frágiles.

Entonces simplemente cambiaría su función original a:

def get_last_appearance_of_keyword(text, keyword):
    line_number = 0
    for line in text:
        if keyword in line:
            line_number = line
    return line_number

Ahora tiene una función que tiene una sola responsabilidad: encontrar la última aparición de una palabra clave en algún texto. Si ese texto proviene de un archivo, se convierte en la responsabilidad del interlocutor. Al realizar la prueba, puede pasar un bloque de texto. Cuando se usa con código de tiempo de ejecución, primero se lee el archivo, luego se llama a esta función. Esa es la separación real de las preocupaciones.

El principio de responsabilidad única establece que una clase debe ocuparse de una sola pieza de funcionalidad, y esta funcionalidad debe encapsularse adecuadamente en su interior.

¿Qué hace exactamente tu método? Obtiene la última aparición de una palabra clave. Cada línea dentro del método funciona para esto y no está relacionada con nada más, y el resultado final es solo uno y uno solo. En otras palabras: no tiene que dividir este método en otra cosa.

La idea principal detrás del principio es que al final no debes hacer más de una cosa. Tal vez abra el archivo y también lo deje así para que otros métodos puedan usarlo, estará haciendo dos cosas. O si persistiera los datos relacionados con este método, nuevamente, dos cosas.

Ahora, puede extraer la línea "abrir archivo" y hacer que el método reciba un objeto de archivo para trabajar, pero eso es más una refactorización técnica que tratar de cumplir con el SRP.

Este es un buen ejemplo de sobre ingeniería. No pienses demasiado o terminarás con un montón de métodos de una línea.

Mi opinión al respecto: depende :-)

En mi opinión, el código debe cumplir con estos objetivos, ordenados por prioridad:

1. Cumplir con todos los requisitos (es decir, hace correctamente lo que debería)
2. Ser legible y fácil de seguir / comprender
3. Ser fácil de refactorizar
4. Seguir buenas prácticas / principios de codificación

Para mí, su ejemplo original supera todos estos objetivos (excepto quizás la corrección debido a lo line_number = lineque ya se mencionó en los comentarios , pero ese no es el punto aquí).

La cuestión es que SRP no es el único principio a seguir. También hay No lo vas a necesitar (YAGNI) (entre muchos otros). Cuando los principios chocan, necesitas equilibrarlos.

Su primer ejemplo es perfectamente legible, fácil de refactorizar cuando lo necesita, pero es posible que no siga SRP tanto como podría.

Cada método en su tercer ejemplo también es perfectamente legible, pero todo esto ya no es tan fácil de entender porque tiene que conectar todas las piezas en su mente. Sin embargo, sigue a SRP.

Como no está obteniendo nada ahora al dividir su método, no lo haga, porque tiene una alternativa que es más fácil de entender.

A medida que cambian sus requisitos, puede refactorizar el método en consecuencia. De hecho, "todo en una cosa" podría ser más fácil de refactorizar: imagine que desea encontrar la última línea que coincida con algún criterio arbitrario. Ahora solo necesita pasar alguna función lambda predicada para evaluar si una línea coincide con el criterio o no.

def get_last_match(file, predicate):
    with open(file, 'r') as file:
        line_number = 0
        for line in file:
            if predicate matches line:
                line_number = line
        return line_number

En su último ejemplo, debe pasar el predicado a 3 niveles de profundidad, es decir, modificar 3 métodos solo para modificar el comportamiento del último.

Tenga en cuenta que incluso dividir la lectura del archivo (una refactorización que generalmente muchos consideran útil, incluido yo) podría tener consecuencias inesperadas: debe leer todo el archivo en la memoria para pasarlo como una cadena al método. Si los archivos son grandes, puede que no sea lo que desea.

En pocas palabras: los principios nunca deben seguirse al extremo sin dar un paso atrás y tener en cuenta todos los demás factores.

¿Tal vez la "división prematura de métodos" podría considerarse un caso especial de optimización prematura ? ;-)

Esto es como una pregunta de equilibrio en mi mente sin una respuesta fácil correcta o incorrecta. Voy a seguir con un enfoque de compartir mis experiencias personales aquí, incluidas mis propias tendencias y errores a lo largo de mi carrera. YMMV considerablemente.

Como advertencia, trabajo en áreas que involucran algunas bases de código a gran escala (millones de LOC, a veces legado de décadas). También trabajo en un área particularmente peculiar donde ninguna cantidad de comentarios o claridad de código necesariamente puede traducirse en que cualquier desarrollador competente pueda entender lo que está haciendo la implementación (no necesariamente podemos tomar un desarrollador decente y lograr que comprenda la implementación de un estado implementación de dinámica de fluidos de última generación basada en un artículo publicado hace 6 meses sin que él pase una buena cantidad de tiempo alejado del código para especializarse en esta área). Esto generalmente significa que solo unos pocos desarrolladores pueden comprender y mantener efectivamente cualquier parte particular de la base de código.

Dadas mis experiencias particulares y tal vez combinadas con la naturaleza peculiar de esta industria, ya no me pareció productivo tomar SoC, DRY, hacer que las implementaciones de funciones fueran lo más legibles posible, incluso la reutilización a sus límites máximos a favor de YAGNI, desacoplamiento, comprobabilidad, pruebas de escritura, documentación de la interfaz (para que al menos sepamos cómo usar una interfaz incluso si la implementación requiere demasiados conocimientos especializados) y, finalmente, enviar el software.

Bloques de lego

En realidad, era propenso a ir en la dirección totalmente opuesta originalmente en algún momento al principio de mi carrera. Me emocioné mucho con la programación funcional y los diseños de clase de política en el diseño moderno de C ++ y la metaprogramación de plantillas, etc. En particular, me entusiasmaron los diseños más compactos y ortogonales donde tienes todas estas pequeñas piezas de funcionalidad (como "átomos") que puedes combinar (para formar "moléculas") en formas aparentemente infinitas para obtener los resultados deseados. Me hizo querer escribir casi todo como funciones que consistían en unas pocas líneas de código, y no hay necesariamente nada intrínsecamente incorrecto con una función tan corta (todavía puede ser muy amplia en su aplicabilidad y aclarar código), excepto que estaba empezando a ir en la dirección dogmática de pensar que mi código tenía algo mal si alguna función abarcaba más de unas pocas líneas. Y obtuve algunos juguetes realmente buenos e incluso un código de producción de ese tipo de código, pero estaba ignorando el reloj: las horas, los días y las semanas que pasaban.

En particular, mientras admiraba la simplicidad de cada pequeño "bloque de lego" que creé que podía combinar de maneras infinitas, ignoré la cantidad de tiempo y la capacidad intelectual que estaba poniendo para juntar todos estos bloques para formar un "artilugio" elaborado. Además, en los raros pero dolorosos casos en que algo salió mal con el elaborado artilugio, ignoré voluntariamente el tiempo que pasaba tratando de averiguar qué salió mal rastreando una serie aparentemente interminable de llamadas a funciones que analizan cada pieza de lego descentralizada y subconjuntos de sus combinaciones cuando todo podría haber sido mucho más simple si no estuviera hecho de estos "legos", por así decirlo, y solo escrito como un puñado de funciones más carnosas o una clase de peso medio.

Sin embargo, di la vuelta al círculo y, como los plazos me obligaron a tomar más conciencia del tiempo, comencé a darme cuenta de que mis esfuerzos me enseñaban más sobre lo que estaba haciendo mal que sobre lo que estaba haciendo bien . Comencé a apreciar una vez más la función más carnosa y el objeto / componente aquí y allá, que hay formas más pragmáticas de lograr un grado razonable de SoC como lo David Arnoseñala la separación del ingreso de archivos del procesamiento de cadenas sin necesariamente descomponer el procesamiento de cadenas al máximo Nivel granular imaginable.

Funciones más carnosas

Y aún más, comencé a estar de acuerdo con incluso una duplicación de código, incluso una duplicación lógica (no estoy diciendo copiar y pegar codificación, todo de lo que estoy hablando es encontrar "equilibrio"), siempre que la función no sea propensa incurrir en cambios repetidos y documentados en términos de su uso y, sobre todo, bien probados para asegurarse de que su funcionalidad coincida correctamente con lo que está documentado para hacer y se mantenga de esa manera. Empecé a darme cuenta de que la reutilización está en gran medida vinculada a la fiabilidad .

Me he dado cuenta de que incluso la función más carnosa que sigue siendo lo suficientemente singular como para no ser aplicable de manera demasiado estricta y demasiado torpe para usar y probar, incluso si duplica alguna lógica en algunas funciones distantes en otras partes de la base de código, y siempre que sea bien probado y confiable, y las pruebas aseguran razonablemente que sigue siendo así, sigue siendo preferible a la combinación más descompuesta y flexible de funciones que carecen de esta calidad. Así que hoy en día me han gustado algunas de las cosas más carnosas si es confiable .

También me parece que la mayoría de las veces, es más barato para darse cuenta de que Está a necesitar algo en retrospectiva y añadirlo, siempre que su código es al menos receptivos a las nuevas adiciones sin derramamiento fuego infernal, que a código de todo tipo de cosas cuando se aren No lo necesitaré y luego enfrentaré la tentación de eliminarlo todo cuando comience a convertirse en un verdadero PITA para mantener.

Así que eso es lo que he aprendido, esas son las lecciones que he considerado más necesarias para que yo aprenda personalmente en retrospectiva en este contexto, y como advertencia, debe tomarse con un grano de sal. YMMV. Pero, con suerte, eso podría ser de algún valor para usted al ayudarlo a encontrar el tipo de equilibrio adecuado para enviar productos que satisfagan a sus usuarios en un período de tiempo razonable y mantenerlos de manera efectiva.

El problema con el que se encuentra es que no está factorizando sus funciones en su forma más reducida. Eche un vistazo a lo siguiente: (No soy un programador de Python, así que no me preocupe)

def lines_from_file(file):
    with open(file, 'r') as text:
        line_number = 1
        lines = []
        for line in text:
            lines.append((line_number, line.strip()))
            line_number += 1
    return lines

def filter(l, func):
    new_l = []
    for x in l:
        if func(x):
            new_l.append(x)
    return new_l

def contains(needle):
    return lambda haystack: needle in haystack

def last(l):
    length = len(l)
    if length > 0:
        return l[length - 1]
    else:
        return None

Cada una de las funciones anteriores hace algo completamente diferente, y creo que le costará más factorizar esas funciones. Podemos combinar esas funciones para realizar la tarea en cuestión.

lines = lines_from_file('./test_file')
filtered = filter(lines, lambda x : contains('some value')(x[1]))
line = last(filtered)
if line is not None:
    print(line[0])

Las líneas de código anteriores se pueden agrupar fácilmente en una sola función para realizar exactamente lo que está buscando hacer. La forma de separar realmente las preocupaciones es dividir las operaciones complejas en su forma más factorizada. Una vez que tenga un grupo de funciones bien factorizadas, puede comenzar a unirlas para resolver el problema más complejo. Una cosa buena de las funciones bien factorizadas es que a menudo son reutilizables fuera del contexto del problema actual en cuestión.

Yo diría que, de hecho, nunca hay demasiada separación de preocupaciones. Pero puede haber funciones que solo use una vez, y ni siquiera las pruebe por separado. Se pueden alinear de forma segura , evitando que la separación se filtre en el espacio de nombres externo.

Su ejemplo literalmente no necesita check_if_keyword_in_string, porque la clase de cadena ya proporciona una implementación: keyword in linees suficiente. Pero puede planear intercambiar implementaciones, por ejemplo, una que use una búsqueda de Boyer-Moore o que permita una búsqueda diferida en un generador; entonces tendría sentido.

Su find_last_appearance_of_keywordpodría ser más general, y encontrar la última aparición de un elemento en una secuencia. Para eso, puede usar una implementación existente o hacer una implementación reutilizable. También puede tomar un filtro diferente , para que pueda buscar una expresión regular, o coincidencias que no distingan entre mayúsculas y minúsculas, etc.

Por lo general, cualquier cosa que se get_text_from_fileocupe de E / S merece una función separada, por lo que puede ser una buena idea si desea manejar varios casos especiales directamente. Puede que no sea así si confía en un IOErrorcontrolador externo para eso.

Incluso el recuento de líneas puede ser una preocupación separada si en el futuro es posible que deba admitir, por ejemplo, líneas de continuación (por ejemplo, con \) y necesite el número de línea lógica. O puede que necesite ignorar las líneas de comentarios, sin romper la numeración de líneas.

Considerar:

def get_last_appearance_of_keyword(filename, keyword):
    with open(filename) as f:  # File-opening concern.
        numbered_lines = enumerate(f, start=1)  # Line-numbering concern.
        last_line = None  # Also a concern! Some e.g. prefer -1.
        for line_number, line in numbered_lines:  # The searching concern.
            if keyword in line: # The matching concern, applied.
                last_line = line_number
    # Here the file closes; an I/O concern again.
    return last_line

Vea cómo es posible que desee dividir su código cuando considere algunas inquietudes que pueden cambiar en el futuro, o simplemente porque advierte cómo se puede reutilizar el mismo código en otro lugar.

Esto es algo a tener en cuenta cuando escribe la función original corta y dulce. Incluso si aún no necesita las preocupaciones separadas como funciones, manténgalas separadas tanto como sea práctico. No solo ayuda a evolucionar el código más tarde, sino que ayuda a comprender mejor el código de inmediato y cometer menos errores.

¿Cuándo es "demasiada" separación? Nunca. No puedes tener demasiada separación.

Su último ejemplo es bastante bueno, pero tal vez podría simplificar el ciclo for con ao text.GetLines(i=>i.containsKeyword)algo.

* Versión práctica: detente cuando funcione. Separar más cuando se rompe.