геттеры и сеттеры, выполняющие дополнительную логику - PullRequest
22 голосов
/ 31 октября 2011

У меня есть класс Java, который представляет корреляцию между двумя элементами (типичный POJO):

public class Correlation {

    private final String a;
    private final String b;

    private double correlation;

    public Correlation(String a, String b) {
        this.a = a;
        this.b = b;
    }

    public double getCorrelation() {
        return correlation;
    }

    public void setCorrelation(double correlation) {
        this.correlation = correlation;
    }

}

Чтобы следовать правильной логике корреляции, если a равно b, тогда значение корреляции должно быть ВСЕГДА 1. Я мог быдобавьте логику, изменяющую метод получения (игнорируйте факт возможного нулевого значения для a):

public double getCorrelation() {
    if (a.equals(b)) {
        return 1D;
    } else {
        return correlation;
    }
}

Что меня беспокоит, так это добавление этой логики в метод получения, если я изменю имя метода или задокументирую егонужно считать достаточно?

Ответы [ 7 ]

15 голосов
/ 31 октября 2011

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

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

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

6 голосов
/ 31 октября 2011

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

Надеюсь, это поможет.

1 голос
/ 31 октября 2011

Учитывая, что корреляция является каким-то / иногда «производным» значением от a и b (т.е. это 1, если a равно b, но это может быть вычислено некоторым оригинальным способом в зависимости от (a, b), хороший вариант можетВы должны рассчитать корреляцию в конструкторе и выдать исключение IllegalArgumentException в setCorrelation, если vaule нарушает внутреннюю логику объекта:

public class Correlation {

    private final String a;
    private final String b;

    private double correlation;

    public Correlation(String a, String b) {
        this.a = a;
        this.b = b;
        calculateCorrelation();
    }

    protected calculateCorrelation() { 
        // open to more complex correlation calculations depending on the input,
        // overriding by subclasses, etc.
        if (a.equals(b)) {
            this.correlation = 1D;
        } else {
            this.correlation = 0;
        }
    }

    public double getCorrelation() {
        return correlation;
    }

    public void setCorrelation(double correlation) throws IllegalArgumentException {
        if (a.equals(b) && correlation != 1D) {
            throw new IllegalArgumentException("Correlation must be 1 if a equals b");
        }

        this.correlation = correlation;
    }
}

Следуя этой схеме, вы также можете «обобщить» свой класс корреляции.

1 голос
/ 31 октября 2011

Имеет смысл применить значение во время setCorrelation(...).Например,

public void setCorrelation(double correlation) {
  if (a.equals(b)) {
    if (Math.abs(correlation - 1D) > EPSILON) {
      throw new InconsistentException(...);
    }
    this.correlation = 1D;
  } else {
    this.correlation= correlation;
  }
}

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

0 голосов
/ 31 октября 2011

Как рассчитывается корреляция в случаях, когда a! = B?

Если корреляция рассчитывается в основном по a и b, setCorrelation () следует удалить. Период. Корреляция должна быть рассчитана в конструкторе или в методе getCorrelation (). Один из принципов ООП заключается в группировании связанных данных и логики, поэтому вычисление корреляции должно в идеале выполняться в классе, который вы ловко назвали Correlation. Если расчет чрезвычайно сложен, он может быть в другом месте (например, DIP), но вызов должен быть сделан из Correlation.

Если корреляция не рассчитывается по a и b, я действительно не понимаю класс, поэтому «это зависит». Если a равно b, и кто-то вызывает setCorrelation (0.0), является ли контракт тихим игнорированием вызова и выходом корреляции на уровне 1.0 или для исключения? И, если я пишу код вызова, я нахожусь в затруднительном положении, так как я не знаю, что произойдет, если я попытаюсь установитьCorrelation (0.0), потому что я понятия не имею, что такое a и b, или везде, где я сделать звонок, я вынужден пойти if (getA().equals(getB())) и т. д. ... или поймать исключение и сделать, э-э, что ??? Что нарушает DRY и именно поэтому ООП говорит, что логика и данные должны быть сгруппированы в классе.

0 голосов
/ 31 октября 2011

Если бы я использовал ваш класс, я бы ожидал, что getCorrelation () вернет корреляцию. На самом деле, я бы, вероятно, перепроектировал бы класс, чтобы иметь статический метод correlateElements (String a, String b), который возвращает значение типа double.

0 голосов
/ 31 октября 2011

Я бы сказал, использовать что-то вроде getValue ()

...