For pr - #115
Conversation
Для задачи было выбрано задание про животных из 13 урока https://github.com/KFalcon2022/lessons/blob/master/lessons/java-core/013/Inheritance.%20Keywords%20extends%20and%20super.%20Access%20modifier%20protected.md Решение задачи с учётом новых знаний об абстрактных классах и интерфейсах.
Для задачи было выбрано задание про животных из 13 урока https://github.com/KFalcon2022/lessons/blob/master/lessons/java-core/013/Inheritance.%20Keywords%20extends%20and%20super.%20Access%20modifier%20protected.md Решение задачи с учётом новых знаний об абстрактных классах и интерфейсах.
| public class Main { | ||
| public static void main(String[] args) { | ||
| static void main() { | ||
| new Main().run(); |
There was a problem hiding this comment.
не логичнее сделать метод статическим? Объект Main-класса не нужен примерно никогда по моему опыту
| } | ||
|
|
||
| private void run(){ | ||
| try(Scanner input = new Scanner(System.in)) { |
There was a problem hiding this comment.
В целом, большого смысла закрывать стандартный поток ввода нет. Не ты открывал, не тебе и закрывать. Я помню, что в статьях это подано несколько иначе, но это было неудачным решением, которое стоит убрать, но не доходят руки
| break; | ||
| case 2: | ||
| // Треугольник | ||
| System.out.print("Укажите длину боковой стороны треугольника: "); |
There was a problem hiding this comment.
для правильного треугольника не может быть понятия боковой стороны, они все равны:)
There was a problem hiding this comment.
Так-то оно так. Просто визуально он не выглядел правильным при рисовании элементами "/" и "". Отсюда и такая формулировка возникла.
| // Квадрат | ||
| System.out.print("Укажите длину стороны квадрата: "); | ||
| Square square = new Square(input.nextInt()); | ||
| square.drawShape(); |
There was a problem hiding this comment.
Собственно, зачем тебе в каждом кейсе вызывать этот метод, если он есть у любого объекта RegularShape?:)
There was a problem hiding this comment.
Тут я в целом уже понял, что это также была одной из целью урока, связанной с полиморфизмом, что мы можем вызвать функцию для переменной типа RegularShape, но реализация уже будет выполняться та, что описана для объекта дочернего классе Square или Triangle.
| protected final String verticalLeftLine; | ||
|
|
||
| // Элемент для рисования правой вертикальной линии | ||
| protected final String verticalRightLine; |
There was a problem hiding this comment.
Ты внес в понятие правильной фигуры специфику ее наследников. Это ошибка проектирования. Что мы будем делать, если завтра добавится правильный пятиугольник?
Кроме того, у правильной фигуры все стороны равны, в нескольких полях просто нет смысла
There was a problem hiding this comment.
UPD. Вижу, это символы, а не длины сторон. Но какой смысл хранить их в родительском объекте? Если что-то является спецификой наследника, почти никогда нет смысла сообщать об этой специфике родителю
|
|
||
| // Защищённый метод рисования фигуры | ||
| protected void draw(){ | ||
| // Функционал будет переопределён в дочерних классах |
There was a problem hiding this comment.
Почему бы не переопределять публичный метод? То, что ты здесь делаешь тоже имеет право на жизнь - см. паттерн template method. Но здесь просто нет пространства для его применения, итого получается оверинжиниринг
| // Для квадрата можно сформировать всего две строки: | ||
| // горизонтальную линию из "---" для верхней и нижней границы фигуры | ||
| // и горизонтальную линию с вертикальными элементами "|" по краям | ||
| String topBottom = getRowLine(this.length, EMPTY_ELEMENT, this.horizontalLine); |
There was a problem hiding this comment.
row line - строка-линия. Идея понятна, но как будто можно сократить до getRow/getLine
| } | ||
|
|
||
| //********************************************************************************// | ||
| // Метод формирует строку заданной длины из указанных элементов для краёв и середины |
There was a problem hiding this comment.
Раз все равно де-факто документация есть - предлагаю ознакомиться с Java-docs. Это чуть иной синтаксис комментариев с поддержкой тегов внутри. Если очень интересно - можно еще и правила документирования посмотреть. Это близко к тому, что ты описываешь здесь, но чуть лаконичнее и привычнее взгляду Java-разработчика.
Как по мне, это очень второстепенная тема, но судя по тому, что вижу - тебе может быть интересно
| } | ||
|
|
||
| // Выводим последнюю строку рисования прямоугольника | ||
| System.out.println(topBottom); |
There was a problem hiding this comment.
каждый sout - обращение к внешнему ресурсу (консоли) с его блокировкой. Как вариант оптимизации - собирать конечную строку со всей фигурой и лишь затем выводить ее в консоль
| } else if (i == rightElement) { // итерация цикла совпала с индексом правого элемента | ||
| strRow.append(this.verticalRightLine); | ||
| } | ||
| else if (this.length * 2 == rightElement) { // Добрались до рисования последней строки треугольника |
There was a problem hiding this comment.
Этот блок точно есть смысл размещать в цикле?
There was a problem hiding this comment.
Тут, да. не очень удачная реализация (так и думал, что тут будут вопросы). В последующих уроках с правильными фигурами, я немного по-другому это место обыграл
|
|
||
| protected final String soundPhrase; | ||
|
|
||
| public Animal(String soundPhrase){ |
There was a problem hiding this comment.
protected. С точки зрения доступности конструктора разница не велика, однако для побочных механизмов - той же интроспекции (например, через Reflection API), публичный конструктор абстрактного класса может оказаться дополнительным корнер кейсом, который лучше просто не добавлять.
Сюда же - просто принятые практики оформления кода, но они на данном этапе не так важны
Вариант решения задания №1 из 14 урока.
https://github.com/KFalcon2022/lessons/blob/master/lessons/java-core/014/Polymorphism.%20Overriding%20method.%20Types%20of%20polymorphism.md