Skip to content

Task19#804

Open
xBobrov wants to merge 3 commits intoKFalcon2022:masterfrom
xBobrov:task19
Open

Task19#804
xBobrov wants to merge 3 commits intoKFalcon2022:masterfrom
xBobrov:task19

Conversation

@xBobrov
Copy link

@xBobrov xBobrov commented Jan 2, 2025

No description provided.

@EvgeneZhurov EvgeneZhurov self-assigned this Jan 3, 2025
public class Parallelepiped {
private int x1;
private int y1;
private int z1;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так-с. Мы описываем фигуру двумя точками в пространстве. И точка выглядит вполне понятной сущностью. Отсюда вопрос: почему мы оперируем не двумя точками, а шестью координатами?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил точку. Но, мне она кажется лишней сущностью. Да, создание параллелепипеда и круга через точки выглядит как будто красивее, но в методе для проверки на пересечение нам нужны именно что шесть координат одной фигуры и три координаты другой. То есть сначала мы задаем их через точку, а затем через точку же и получаем. Может я что-то не так понял.

}

public static Parallelepiped getInstance(int x1, int y1, int z1, int x2, int y2, int z2) {
if (x1 == x2 || y1 == y2 || z1 == z2) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

что мешает засунуть эту проверку в конструктор, если считаем, что она нужна?

Ну и getInstance() - почти общепринятое название метода при реализации наивного синглтона. Если хочется именно через метод создавать - достаточно распространенное название для него будет of()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

На просторах stackoverflow наткнулся на дискуссию о проверках при создании экземпляра класса и выбрал что больше понравилось.
Распространённое название getInstanceOf() или просто of() ? Уточняю перед тем, как исправить.


private int x2;
private int y2;
private int z2;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если считаем, что поля неизменяемы (нет сеттеров) - почему бы не пометить их как final?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

пометил

public class Sphere {
private int x;
private int y;
private int z;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тот же вопрос про сущность "точка", что и выше


public static Parallelepiped getInstance(int x1, int y1, int z1, int x2, int y2, int z2) {
if (x1 == x2 || y1 == y2 || z1 == z2) {
return null;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

исключения не проходили, поэтому такой формат как будто ок. Но на будущее - в таких ситуациях лучше выбросить исключение:)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Приму к сведению, исправлять не стал.

static boolean isInteractedProjected(int rectangleX1, int rectangleY1, int rectangleX2, int rectangleY2,
int circleX, int circleY, int circleRadius) {

int rectangleTop = Math.max(rectangleY1, rectangleY2);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

момент на подумать: не лучше ли при создании параллелепипеда определять взаимное положение точек в пространстве любым удобным способом и на базе этого присваивать значение условным "верхней" и "нижней" точкам?

Учитывая, что речь о проекциях и в них вопрос верхней/нижней может отличаться от рассматриваемой плоскости - мб идея тупиковая, но рассмотреть стоит

Опять же, мб имеет смысл засунуть получение верхней/нижней координаты в API параллелепипеда, это выглядит более функциональным, чем базовые геттеры

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сделал через массивы, где первый элемент верхняя или правая координата, второй нижняя или левая координата для прямоугольника и координаты центра проекции для окружности.


static boolean isVertexIntersectCircle(int vertexX, int vertexY, int circleX, int circleY, int circleRadius) {

return circleRadius * circleRadius >= (vertexX - circleX) * (vertexX - circleX)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут бы части выражения в переменные вынести. Ну и Math.pow() точно читабельнее, чем возведение в квадрат через перемножение. Не говоря о том, что условная переменная deltaX понятнее, чем двойной расчет разности

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Согласен, получилось аккуратнее.

@xBobrov xBobrov requested a review from KFalcon2022 January 8, 2025 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments