2016-05-22 2 views
1

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

public class Adder { 
    public int sum(int[] array) { 
    int result = 0; 
    for (int i = 0; i < array.length; i++) { 
     result += array[i]; 
    } 
    return result; 
} 

public class Multiplier { 
    public int multiply(int[] array) { 
    int result = 1; 
    for (int i = 0; i < array.length; i++) { 
     result *= array[i]; 
    } 
    return result; 
} 

Обращаем внимание на другую начальную страницу: result; это моя главная проблема.

+0

Почему бы не пройти начальное значение как параметр в одном общем методе? – Turing85

+1

Существует две разные операции. Я не думаю, что здесь много дублирования. Я бы сохранил это как есть. Btw, Adder и Multiplier звучат немного искусственно для меня. Почему бы не создать класс калькулятора с методом sum и multiply? –

+0

Я думаю, что фактическая операция должна быть изолирована от цикла for, но 'result' не находится в том же объеме. –

ответ

1

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

public abstract class ArrayFuntion { 

    public int compute(int[] array) { 
     int result = initResult(); 
     for (int i = 0; i < array.length; i++) { 
      result = compute(result, array[i]); 
     } 
     return result; 
    } 

    protected abstract int compute(int result, int i); 

    protected abstract int initResult(); 

} 


public class Adder extends ArrayFuntion{ 

    @Override 
    protected int compute(int result, int arrayItem) { 
     return result + arrayItem; 
    } 

    @Override 
    protected int initResult() { 
     return 0; 
    } 

} 


public class Multiplier extends ArrayFuntion { 

    @Override 
    protected int compute(int result, int arrayItem) { 
     return result * arrayItem; 
    } 

    @Override 
    protected int initResult() { 
     return 1; 
    } 

} 
+0

Молодцы. С интерфейсом сверху это будет еще лучше. –

2

Если вы действительно думаете, это требует некоторого рефакторинга рассмотрит это:

public class Calculator { 
    public int multiply(int[] array) { 
     return calculate(1, array, (a, b) -> a * b); 
    } 

    public int sum(int[] array) { 
     return calculate(0, array, (a, b) -> a + b); 
    } 

    public int calculate(int initValue, int[] array, IntBinaryOperator operator) { 
     return Arrays.stream(array).reduce(initValue, operator); 
    } 

    public static void main(String[] args) { 
     Calculator calculator = new Calculator(); 
     System.out.println(calculator.multiply(new int[]{1, 2, 3, 4})); 
     System.out.println(calculator.sum(new int[]{1, 2, 3, 4})); 
    } 
} 
+0

Очень сильный и лаконичный, но imho он вкладывает слишком много энергии в руки программиста и меньше в дизайнере. –

+0

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

2

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

public abstract class CommonMath { 
    public int calculate(int initialValue, int[] array) { 
     int result = initialValue; 
     for (int i = 0; i < array.length; i++) { 
      result = mathOperation(result, array[i]); 
     } 
     return result; 
    } 

    public abstract int mathOperation(int result, int arrayItem); 
} 

public class Adder extends CommonMath { 
    public int sum(int[] array) { 
     return calculate(0, array); 
    } 

    @Override 
    public int mathOperation(int result, int arrayItem) { 
     return result + arrayItem; 
    } 
} 

public class Multiplier extends CommonMath { 
    public int multiply(int[] array) { 
     return calculate(1, array); 
    } 

    @Override 
    public int mathOperation(int result, int arrayItem) { 
     return result * arrayItem; 
    } 
} 

// test 
public static void main(String[] args) { 
    try { 
     int[] array; { 
      array = new int[3]; 
      array[0] = 1; 
      array[1] = 2; 
      array[2] = 4; 
     } 
     System.out.println("sum " + Arrays.toString(array) + " " + new Adder().sum(array)); 
     System.out.println("multi " + Arrays.toString(array) + " " + new Multiplier().multiply(array)); 
    } catch (Exception e) { 
     e.printStackTrace(); 
    } 
} 

выход

sum [1, 2, 4] 7 
multi [1, 2, 4] 8 
0

Как насчет

public abstract class Calculator { 
    protected int aggregate(int[] array, int startValue) { 
     int result = startValue; 
     for (int i = 0; i < array.length; i++) { 
      result = this.aggregateSingle(array[i], result); 
     } 
     return result; 
    } 

    protected abstract int aggregateSingle(int nextValue, int oldAggregation); 
} 

public class Adder extends Calculator { 
    public int sum(int[] array) { 
     return this.aggregate(array, 0); 
    } 
    protected int aggregateSingle(int nextValue, int oldAggregation) { 
     return nextValue + oldAggregation; 
    } 
} 

public class Multiplier extends Calculator { 
    public int multiply(int[] array) { 
     return this.aggregate(array, 1); 
    } 
    protected int aggregateSingle(int nextValue, int oldAggregation) { 
     return nextValue * oldAggregation; 
    } 
} 

Этот подход даже сохраняет структуру класса, что может быть важно в том случае, классы Adder и Multiplier используются снаружи (они public!)

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

+0

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

Смежные вопросы