Меньшие методы против метода четкой рекурсии

Question

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

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

Итак, что вы думаете по этому поводу? Я включаю оба подхода ниже.

Это метод одного метода.

private object GetObject(string propertyName, object dataObject) 
{ 
    object returnObject; 

    if (dataObject == null) return null; 

    if (propertyName.Contains(".")) 
    { 
    Type dataObjectType = dataObject.GetType(); 
    PropertyInfo dataObjectProperty = dataObjectType.GetProperty(propertyName.Substring(0, propertyName.IndexOf("."))); 

    ThrowExceptionIfObjectOrPropertyInexistant(dataObjectProperty, dataObjectType.FullName, propertyName); 

    object nestedObject = dataObjectProperty.GetValue(dataObject, null); 
    string nestedObjectPropertyName = propertyName.Substring(propertyName.IndexOf(".") + 1); 

    returnObject = GetObject(nestedObjectPropertyName, nestedObject); 
    } 
    else 
    { 
    returnObject = dataObject; 
    } 

    return returnObject; 
} 

Это два метода один.

private object GetObject(string propertyName, object dataObject) 
{ 
    object returnObject; 

    if (dataObject == null) return null; 

    if (propertyName.Contains(".")) 
    { 
    returnObject = GetNestedObject(propertyName, dataObject); 
    } 
    else 
    { 
    returnObject = dataObject; 
    } 

    return returnObject; 
} 

private object GetNestedObject(string propertyName, object dataObject) 
{ 
    object returnObject; 
    Type dataObjectType = dataObject.GetType(); 
    PropertyInfo dataObjectProperty = dataObjectType.GetProperty(propertyName.Substring(0, propertyName.IndexOf("."))); 

    ThrowExceptionIfObjectOrPropertyInexistant(dataObjectProperty, dataObjectType.FullName, propertyName); 

    object nestedObject = dataObjectProperty.GetValue(dataObject, null); 
    string nestedObjectPropertyName = propertyName.Substring(propertyName.IndexOf(".") + 1); 

    returnObject = GetObject(nestedObjectPropertyName, nestedObject); 
    return returnObject; 
} 

Answer 1

Как о версии 1-метода с четким караульным пунктом в верхней части, и с рекурсией четко видна на хвостовом конце методы:

private object GetObject(string propertyName, object dataObject) 
{ 
    int index = propertyName.IndexOf("."); 
    if (dataObject == null || index == -1) 
    { 
    return dataObject; 
    } 

    Type dataObjectType = dataObject.GetType(); 
    PropertyInfo dataObjectProperty = dataObjectType.GetProperty(propertyName.Substring(0, index)); 

    ThrowExceptionIfObjectOrPropertyInexistant(dataObjectProperty, dataObjectType.FullName, propertyName); 

    return GetObject(propertyName.Substring(propertyName.IndexOf(".") + 1), 
        dataObjectProperty.GetValue(dataObject, null)); 
} 

Создания переменных для одноразового использования, иногда просто создает более семантический «шум», который человек должен анализировать, пытаясь понять код. Я «рухнул» несколько одноразовых переменных, которые, я считаю, уточняют, что делает рекурсия. YMMV. Я также вычислил фронт index, поэтому вам нужно всего лишь один раз проверить строку, чтобы увидеть, содержит ли она символ '.', а не дважды.

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

Answer 2

Поскольку кто-то дал мне минус один, я думаю, мне нужно уточнить. Вот что я имел в виду:

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

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

private object GetInnermostObject(object dataObject, string propertyName){ 
     object currentObject = dataObject; 
     foreach(propName in propertyName.Split(".")) currentObject = GetNestedObject(currentObject, propName); 
     return currentObject; 
    } 

    private object GetNestedObject(object dataObject, string propertyName) 
    { 
     PropertyInfo property = GetDataObjectProperty(dataObject.GetType(), propertyName); 
     object nestedObject = property.GetValue(dataObject, null); 
     return new DataObjectWrapper(nestedObject); 
    } 

    private static PropertyInfo GetDataObjectProperty(Type dataObjectType, string propertyName){ 
     PropertyInfo dataObjectProperty = dataObjectType.GetProperty(propertyName); 
     ThrowExceptionIfObjectOrPropertyInexistant(dataObjectProperty, dataObjectType.FullName, propertyName); 
     return dataObjectProperty; 
    } 

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

Answer 3

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

1 GetObject 
2 GetNested Object 
3 GetObject 
4 GetNestedObject 
.... 

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

Ваш первый метод вписывается в один экран; Я думаю, что это достаточно короткое, а другой выбор - немного запутанный.

Answer 4

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

Answer 5

У Роберта Мартина есть некоторые чрезвычайно хорошие моменты, сделанные до смешного экстремала.

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

Возьмите «правила» как это в качестве руководства и попробуйте применить их так, чтобы исходное намерение (четкий, сжатый код) не было потеряно.

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

if (!propertyName.Contains(".")) 
    return dataObject; // this is the one! 

// check nested stuff 

Answer 6

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

Это правило применяется еще больше, когда ваш коллега - психопат, который знает, где вы живете. ; o)

Answer 7

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

Я бы сказал, что функция должна делать одно и только одно, но не менее одного.

Answer 8

Ваш сверстник имеет точку, в которой рекурсия скрыта. Труднее выполнить два метода, которые являются взаимно рекурсивными, чем один рекурсивный метод.

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

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

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

private object GetObject(string propertyName, object dataObject) { 
    while (dataObject != null && propertyName.Contains(".")) { 
     Type objectType = dataObject.GetType(); 
     string[] names = propertyName.Split('.', 2); 
     PropertyInfo property = objectType.GetProperty(names[0]); 

     ThrowExceptionIfObjectOrPropertyInexistant(property, objectType.FullName, propertyName); 

     dataObject = property.GetValue(dataObject, null); 
     propertyName = names[1]; 
    } 
    return dataObject; 
} 

Answer 9

Я хотел бы выступать за следующую позицию:

Processes and methodologies can make good servants but are poor masters. 
    - Mark Dowd, John McDonald & Justin Schuh 
    in "The Art of Software Security Assessment" 

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