Разделение проблем: когда это «слишком много» разделение?

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

Когда это слишком много «разделения проблем» относительно
методов?

Допустим, у нас есть следующий метод:

```
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

```

Я думаю, что этот метод в порядке. Это просто, легко читается, и
это ясно, что говорит это имя. Но: На самом деле это не совсем
«одно дело». Он фактически открывает файл, а затем находит его. Это
означает, что я мог бы разделить его еще дальше (также учитывая
«принцип единой ответственности»):

Вариация B (Ну, это как-то понятно). Таким образом, мы можем
легко использовать алгоритм поиска последнего появления ключевого
слова в тексте, но он кажется «слишком большим». Я не могу
объяснить, почему, но я просто «чувствую «это так):

```
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

```

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

```
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

```

Итак, теперь мой вопрос: каков правильный способ написания этого
кода и почему другие подходы правильные или неправильные? Я всегда
учился: Разделение, но никогда, когда это просто слишком много. И
как я могу быть уверен в будущем, что он «прав» и что ему больше не
нужно разделять, когда я снова кодирую?

Ваши различные примеры разделения проблем на отдельные функции
связаны с одной и той же проблемой: вы все еще жестко кодируете
зависимость файла в `get_last_appearance_of_keyword` .
Это делает эту функцию трудной для тестирования, так как теперь она
должна отвечать на файл, существующий в файловой системе при
запуске теста. Это приводит к хрупким испытаниям.

Поэтому я просто изменил бы свою оригинальную функцию:

```
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

```

Теперь у вас есть функция, которая имеет только одну
ответственность: найти последнее вхождение ключевого слова в
некотором тексте. Если этот текст должен поступать из файла, это
становится ответственностью вызывающего абонента. При тестировании
вы можете просто передать блок текста. При использовании его с
кодом времени выполнения сначала считывается файл, затем вызывается
эта функция. Это реальное разделение проблем.



Когда это «слишком много» разделения? Никогда. У вас не может
быть слишком много разлуки.

Your last example is pretty good, but you could maybe simplify
the for loop with a
`text.GetLines(i=>i.containsKeyword)` or
something.

* Практическая версия: Остановитесь, когда это сработает.
Отделите больше, когда он сломается.



Я бы сказал, что на самом деле никогда не бывает слишком много
проблем. Но могут быть функции, которые вы используете только один
раз, и даже не тестируете отдельно. Они могут быть безопасно
*inlined,* , сохраняя разделение от просачивания во внешнее
пространство имен.

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

Ваш `find_last_appearance_of_keyword` может быть
более общим и найти последнее появление элемента в
последовательности. Для этого вы можете использовать существующую
реализацию или выполнить повторную реализацию. Также может
потребоваться другой фильтр , чтобы вы могли искать регулярное
выражение или совпадения с учетом регистра и т. Д.

Обычно все, что имеет дело с I/O, заслуживает отдельной функции,
поэтому `get_text_from_file` может быть хорошей идеей,
если вы хотите обрабатывать различные специальные случаи. Возможно,
вы не полагаетесь на внешний обработчик `IOError` для
этого.

Даже подсчет строк может быть отдельной проблемой, если в
будущем вам может понадобиться поддержка, например. (например, с
помощью `\` ) и потребуется номер логической строки. Или
вам может потребоваться игнорировать строки комментариев, не
нарушая нумерацию строк.

Рассматривать:

```
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

```

Посмотрите, как вы можете *хотеть* разделить свой код,
когда будете рассматривать некоторые проблемы, которые могут
измениться в будущем, или просто потому, что вы заметили, как один
и тот же код можно повторно использовать в другом месте.

Это то, что нужно знать, когда вы пишете оригинальную короткую и
сладкую функцию. Даже если вам еще не нужны проблемы, разделенные
как функции, держите их в отдельности настолько же практично. Это
не только помогает в дальнейшем генерировать код, но помогает лучше
понять код сразу и сделать меньше ошибок.



Проблема, с которой вы сталкиваетесь, заключается в том, что вы
не факторизуете свои функции в их наиболее сокращенной форме.
Взгляните на следующее: (Я не программист на питоне, так что
сократите мне слабину)

```
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

```

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

```
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])

```

Вышеупомянутые строки кода могут быть легко объединены в одну
функцию для выполнения именно того, что вы хотите сделать. Способ
действительно разделить озабоченность состоит в том, чтобы
разрушить сложные операции в их наиболее факторизованную форму.
Когда у вас есть группа хорошо учтенных функций, вы можете начать
собирать их вместе, чтобы решить более сложную проблему. Одна
хорошая вещь о хорошо учтенных функциях состоит в том, что они
часто могут повторно использоваться вне контекста текущей
проблемы.



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

Что именно делает ваш метод? Он получает последнее появление
ключевого слова. Каждая строка внутри метода работает в этом
направлении, и она не связана ни с чем другим, а конечный результат
- только один и один. Другими словами: вам не нужно разделить этот
метод на что-либо еще.

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

Теперь вы можете извлечь строку «открыть файл» и заставить метод
получать файл-объект для работы, но это скорее технический
рефакторинг, чем попытка выполнить SRP.

Это хороший пример более инженерного. Не думайте слишком много,
или вы получите кучу однострочных методов.



Я принимаю это: это зависит :-)

По моему мнению, код должен соответствовать этим целям,
упорядоченным по приоритету:

1. Выполнять все требования (т. е. правильно делает то, что
должно)
2. Быть читаемым и легко следовать/понимать
3. Будьте легко рефакторированы
4. Следуйте хорошим правилам/принципам кодирования.

Для меня ваш оригинальный пример передает все эти цели (за
исключением, может быть, правильности из-за `line_number =
line` , уже упомянутой в [комментарии](https://softwareengineering.stackexchange.com/questions/378914/separation%20of-concern-when-is-it-too-much-separate%20#%20comment833656_378914) , но здесь дело не в этом).

Дело в том, что SRP - не единственный принцип, которым нужно
следовать. Существует также [вам
не понадобится (YAGNI)](https://en.wikipedia.org/wiki/YAGNI) (среди многих других). Когда принципы
сталкиваются, вам необходимо сбалансировать их.

Ваш первый пример отлично читается, легко рефакторируется, когда
вам нужно, но может не следовать SRP как можно больше.

Каждый метод в вашем третьем примере также отлично читается, но
все это уже не так легко понять, потому что вы должны подключить
все части вместе в своем уме. Однако он следит за SRP.

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

По мере изменения ваших требований вы можете реорганизовать
метод соответствующим образом. Фактически, «все в одном» может быть
*проще* для рефакторирования: представьте, что вы хотите
найти последнюю строку, соответствующую произвольному критерию.
Теперь вам просто нужно передать некоторую функцию
лямбда-предиката, чтобы оценить, соответствует ли строка критерию
или нет.

```
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

```

В последнем примере вам нужно пройти глубокий уровень предикатов
3, т. Е. Изменить 3 метода только для изменения поведения
последнего.

Обратите внимание, что даже расщепление чтения файла
(рефакторинг, который, как правило, многие считают полезным,
включая меня) может иметь неожиданные последствия: вам нужно
прочитать весь файл в памяти, чтобы передать его как строку для
вашего метода. Если файлы большие, которые могут быть не такими,
как вы хотите.

Итог: Принципы никогда не должны следовать до крайности, не
делая шаг назад и принимая во внимание все другие факторы.

Возможно, «преждевременное расщепление методов» можно
рассматривать как частный случай [преждевременной оптимизации](http://wiki.c2.com/?PrematureOptimization) ? ;-)