Что плохого в моем коде тестового задания на java?

250

17 июля 2017, 14:30

Задача тестового: привести примеры использования ООП Что подтянуть? Критикуйте пожоще.

maven проект, 4 класа , unit тесты

https://github.com/Sharabaddin/oop-examples

Описания присутствует.

Вопрос задан согласно правилам о подобных вопросах https://ru.stackoverflow.com/tags/%D0%B8%D0%BD%D1%81%D0%BF%D0%B5%D0%BA%D1%86%D0%B8%D1%8F-%D0%BA%D0%BE%D0%B4%D0%B0/info

Answer 1

По условиям задачи, вам для сущности "Конденсатор" необходимы 3(4) метода.
- получить стоимость производства конденсатора. calculateCost()
- Задав характеристику емкости будушего конденсатора. 1 или 2 сеттера
- получить количество нужного материала для производстваю calculateMaterials()

У вас же на каждый подкласс по 7 публичных методов.

Три ваших типа конденсаторов наследуют от одного абстрактного класса в котором... нет ни одного реализованного метода! Т.е. его можно смело рефакторить в интерфейс, но и тут не все гладко. В этот класс у вас зачем-то вынесены только сеттеры и геттеры емкости (зачем геттеры!?), а где остальные 2 метода, про стоимость и материалы?
Сеттеры - это такие методы которые, по феншую, не должны ничего возвращать. Максимум, что может себе позволить сеттер - выбросить исключение в случае неадекватных входных данных.
Возвращаясь к абстрактному классу. У вас во всех наследниках присутствуют абсолютно идентичные методы setPicofarad, setStatfarad (и их геттеры, которые не нужны) которые то и необходимо было реализовать в классе предке.
В расчете материалов (который у вас выполнен вообще странно как-то) вы используете константы. В расчете стоимости захардкоженные цифры. Косяк.
Много лишнего кода (не говоря о том, что вообще не так надо было делать)

Вот это:

public boolean setPicofarad(int picofarad) {
    if (picofarad >= MIN_PICOFARAD && picofarad <= MAX_PICOFARAD) {
        this.picofarad = picofarad;
        return true;
    } else {
        return false;
    }
}
public boolean setStatfarad(int statfarad) {
    if ((statfarad >= 3 * MIN_PICOFARAD) && (statfarad <= 3 * MAX_PICOFARAD)) {
        picofarad = 3 * statfarad;
        return true;
    } else {
        return false;
    }
}

Должно как минимум выглядеть как-то так:

public boolean setPicofarad(int picofarad) {
    if (picofarad < MIN_PICOFARAD || picofarad > MAX_PICOFARAD) {
        return false;
    }
    this.picofarad = picofarad;
    return true;
}
public boolean setStatfarad(int statfarad){
    return setPicofarad(statfarad * 3);
}