Евгений Собко. Кубик Рубика #12
Conversation
Changelog: -a lot of bugs with rotate side -fix rotate cube
src/main/java/myClass/Cube.java
Outdated
| @@ -0,0 +1,178 @@ | |||
| package myClass; | |||
There was a problem hiding this comment.
Не самое лучшее название для пакета. Название должно описывать, что именно содержится в пакете.
src/main/java/myClass/Cube.java
Outdated
|
|
||
| public class Cube { | ||
| public char[][][] cube; | ||
| public int n; |
src/main/java/myClass/Cube.java
Outdated
| public char[][][] cube; | ||
| public int n; | ||
|
|
||
| char[] symbols = {'W', 'R', 'B', 'O', 'Y', 'G'}; |
There was a problem hiding this comment.
К вашей задаче очень хорошо подойдет Enum для описания цветов.
src/main/java/myClass/Cube.java
Outdated
|
|
||
| public Cube(int n) { | ||
| this.n = n; | ||
| this.cube = new char[6][n][n]; |
There was a problem hiding this comment.
Выносите "магические" константы в отдельные final переменные.
src/main/java/myClass/Cube.java
Outdated
|
|
||
| for (int i = 0; i < 6; ++i) { | ||
| for (int j = 0; j < n; ++j) { | ||
| Arrays.fill(cube[i][j], symbols[i]); |
There was a problem hiding this comment.
Напишите комментарий, чтобы сразу было понятно, что на каждой грани будут квадраты одного цвета.
src/main/java/myClass/PrintCube.java
Outdated
| @@ -0,0 +1,57 @@ | |||
| package myClass; | |||
|
|
|||
| public class PrintCube { | |||
There was a problem hiding this comment.
А вот классы обычно обозначаются существительными. CubePrinter тогда уж.
| private int n; | ||
|
|
||
| PrintCube(int n) { | ||
| this.arr = new char[n * 8 + 1][n * 18 + 1]; |
| char[][][] ptr = cube.Get(); | ||
| for (int i = 0; i < n; ++i) { | ||
| for (int j = 0; j < n; ++j) { | ||
| arr[i * 2 + 1][n * 6 + 3 + j * 6] = ptr[0][i][j]; |
There was a problem hiding this comment.
Вот где-где, а тут я "магии" совсем не ожидал
src/main/java/myClass/Cube.java
Outdated
| RotateUp(); | ||
| } | ||
|
|
||
| public void Spin(int t) { |
There was a problem hiding this comment.
Что означает t? Если я буду пользователем вашего класса, как я могу догадаться, что означает данный аргумент?
There was a problem hiding this comment.
Поправил имя переменной, это направление поворота.
There was a problem hiding this comment.
Как задается направление? Если я хочу повернуть грань влево, что мне нужно записать в direction? 10? 1? 0?
| @Test | ||
| public void rotateUp() { | ||
| result.RotateUp(); | ||
| assertArrayEquals(result.cube[0], actual.cube[2]); |
There was a problem hiding this comment.
Опять магия. Надеюсь, когда сделаете стороны через Enum, будет понятнее.
There was a problem hiding this comment.
Аналогично тому что я писал ранее, у нас есть 2 грани (result и actual) для сравнения, actual - измененная грань.
| @@ -0,0 +1,57 @@ | |||
| package myRubiksCube; | |||
|
|
|||
| public class CubePrinter { | |||
There was a problem hiding this comment.
Давайте подумаем, как можно разгрузить этот класс от "магии". Я вижу два варианта.
- Итак, у нас в изображении-развертке 12 блоков (6 граней куба плюс 6 пустых блоков). Можно сделать метод, который будет заполнять один конкретный блок. То есть на выходе он будет давать строку, которая описывает блок. Соединить потом блоки - не проблема.
Для каждого блока мы делаем сетку. Вынесем это тоже в отдельный метод. Можно опять же создать одну ячейку, а потом "размножить" её путем склеивания строк. Заполнение ячейки - тоже отдельный метод, пусть и совсем маленький. Путем такого разбиения можно постепенного избавиться от магических констант. - Создать текстовый файл, который будет содержать шаблон. Можно оставить текущий внешний вид, можно сделать вид а-ля проекция, используя, например, символы
/. Вместо цветов на гранях - буквы. Цифр, заглавных и строчных букв хватит, чтобы отметить все квадраты разными символами. Потом, используяreplace, подставляем вместо символов нужные цвета.
Если бы было побольше времени, можно было бы аналогичным образом даже сформировать картинку.
Есть ещё такой вариант. Вы не убираете "магию" из кода, не переделываете char на enum, исправляете оставшиеся замечания, но оценка будет 4.
There was a problem hiding this comment.
Не видел Вашего комментария, я подумаю что можно с этим сделать.
|
Пока что 4. Задание непростое, вы его решили. Но запутанный код - это бомба замедленного действия. |
|
Если вы болеете, то я готов подождать исправлений. Выздоравливайте! |
|
Понимая объем кода, который мне придется переписать, и учитывая мое текущее состояние, сомневаюсь, что смогу что-либо изменить. Плюс, не могу заставлять ждать, да и самому хочется перейти к следующему заданию. Понимаю, что код запутан очень сильно, постараюсь впредь писать понятнее. Постараюсь поскорее поправиться, спасибо! |
2 participants
Now it works!