Как правильно использовать абстрактные методы?
Всем привет. Нужно мнение опытных разработчиков 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();
}
У него есть наследники:
- PhisicalPerson - переопределяет getInfo();
- IndividualBusinessman - переопределяет getInfo() и put();
- LegalPerson - переопределяет getInfo() и take();
Я сделал абстрактным только один метод getInfo(), чтобы избежать дублирование кода в наследниках, но преподаватель со мной не согласен. А теперь вопрос - как правильно распределять назначение абстрактных методов в таком случае? Может есть какая-то статья по этому поводу?
Ответы (1 шт):
У вас в вашем примере плохо все, потому нет смысла говорить о какой то одной мелочи. Давайте разберем ваш пример.
Начем с названий.
Почему класс называется 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("Неправильная сумма");
}
}
Вопросы:
Почему метод, который предназначем для увеличения суммы на счету, вдруг начинает что то выводить в консоль? Вы читали про SRP (это буква S из SOLID)? Почему вообше клиент занимается выводом в консоль?
Вы проверяете взходной параметр
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 вообще кому попало и вы не можете гарантировать ничего.
Я надеюсь теперь стало понятно, почему я вам в комментариях написал, что здесь не ясна ни ваша задача, ни ваш вопрос. У кода, что вы написали, есть горазо большие проблемы, чем какая то абстрактная функция.