Как правильно использовать абстрактные методы?

Всем привет. Нужно мнение опытных разработчиков Java. У меня есть абстрактный класс Client

public abstract class Client {
    protected double amount;

    double getAmount() {
        return amount;
    }

    public void put(double amountToPut) {
        if (amountToPut > 0) {
            amount += amountToPut;
            System.out.println(amount);
        } else {
            System.out.println("Неправильная сумма");
        }
    }

    protected void take(double amountToTake) {
        if (amountToTake < amount) {
            amount -= amountToTake;
        } else {
            System.out.println("Неправильная сумма");
        }
    }

    public abstract void getInfo();

}

У него есть наследники:

  1. PhisicalPerson - переопределяет getInfo();
  2. IndividualBusinessman - переопределяет getInfo() и put();
  3. LegalPerson - переопределяет getInfo() и take();

Я сделал абстрактным только один метод getInfo(), чтобы избежать дублирование кода в наследниках, но преподаватель со мной не согласен. А теперь вопрос - как правильно распределять назначение абстрактных методов в таком случае? Может есть какая-то статья по этому поводу?


Ответы (1 шт):

Автор решения: tym32167

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

Начем с названий.

Почему класс называется Client? В чем назначение этого класса? Чтобы быть базовым классом для других классов-клиентов? Тогда почему не BaseClient?

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

Следующее, protected double amount; - это количество чего? Яблок и груш? Или денег? Если это количество денег (moneyAmount?), то это суммарное их количество на счете? (и почему вообще сумма денег на счете хранится в клиенте, а не в банке?) Или у вас тут нет счетов? Ок, допустим, у вас нет счета и это количество денег, в какой они валюте? Ок, допустим валюта не важна, тогда почему это число с поаващей точкой? У вас может быть 0.0000001 рубль на счету? Название и тип этого поля нестолько непонятные, что читать этот код скорее вредно, чем полезно.

Дальше, смотрим на методы public void put(double amountToPut) и protected void take(double amountToTake). Имя параметра говорит о передаваемых данных, имя функции говорит о выполняемом действии. То есть нет необходимости в имени параметра писать, что вы делать собрались, потому если вам надо сделать put некоторого amount, так и пишите public void put(double amount). Но даже это не лучший вариант. А что, если у вас будет 2 поля типа double в классе? Как вы будете между ними разгранничивать операции? public void put(double amount1) и public void put(double amount2) работать не будет для разных полей. Потому, возвращаемся к Имя параметра говорит о передаваемых данных, имя функции говорит о выполняемом действии. - ваши операции будут выглядеть как public void putAmount(double value) и public void takeAmount(double value).

Смотрим следующую функцию. public abstract void getInfo(); - вот я читаю первый раз ваш код. Что я из этой строчки должен понять? Вы информацию о чем возврашаете? О фазе луны? И почему функция, которая называется "получитьИнформацию" не возвращает ничего? Вы спрашиваете как её надо было написать, но я лично не понимаю зачем она вообще тратит место в вашем коде.

Остальные названия вашиз классов - PhisicalPerson, IndividualBusinessman, LegalPerson - это все клиенты? Тогда почему не PhysicalPersonBankClient?

Доступ

В вашем коде просто мешанина уровней доступа. У вас поле amount сделано защищенным, а getAmount() - package private. Это зачем?

У вас операция put - публичная, а take - защищенная, в чем тут смысл?

Логика

Смотрим на ваш метод put.

public void put(double amountToPut) {
    if (amountToPut > 0) {
        amount += amountToPut;
        System.out.println(amount);
    } else {
        System.out.println("Неправильная сумма");
    }
}

Вопросы:

  1. Почему метод, который предназначем для увеличения суммы на счету, вдруг начинает что то выводить в консоль? Вы читали про SRP (это буква S из SOLID)? Почему вообше клиент занимается выводом в консоль?

  2. Вы проверяете взходной параметр amountToPut > 0, ок, это хорошо. Но почему при неверном входном параметре вы опять пишете в консоль? Вот я запущу ваш класс, вызову метод, а консоли у меня нет - как мне понять, сделал ли метод свою работу? Вы слышали про исключения?

DRY

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

public void changeAmount(double value)
{
    double newAmount = amount + value; 
    if (newAmount > 0) amount = newAmount;
    else throw new InvalidArgumentException("amount cannot be less than zero");
}

Ок, если вы хотие поддерживать и валидировать операции put и take, вам не кажется, что они обе НЕ должны работать напрямую с полем amount?

public void put(double amountToPut) {
    if (amountToPut > 0) {            
        changeAmount(-amountToPut);
        System.out.println(getAmount());
    } else {
        System.out.println("Неправильная сумма");
    }
}

protected void take(double amountToTake) {
    if (amountToTake < getAmount()) {
        changeAmount(-amountToTake);
    } else {
        System.out.println("Неправильная сумма");
    }
}

Использование функций типа changeAmount очень важно, так как имея единую точку доступа к изменению значения поля amount вы держите ее под контролем. Код выше гарантирует, что amount не станет меньше 0 ни при каких обстоятельствах. Ваш же код, особенно вот это protected double amount; - позволяет менять поле amount вообще кому попало и вы не можете гарантировать ничего.

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

→ Ссылка