Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

init project #1

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

init project #1

wants to merge 10 commits into from

Conversation

leralushnikova
Copy link
Owner

No description provided.

@leralushnikova
Copy link
Owner Author

the fist StudentProject

Copy link

@antonkupreychik antonkupreychik left a comment

Choose a reason for hiding this comment

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

Все отлично в целом, только добей до результат REST и другие сущности:
Про мапстракт - https://stackoverflow.com/questions/60868345/map-abstract-class-to-dto-with-mapstruct

Choose a reason for hiding this comment

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

кажется этот файл тут не нужен?))

@@ -0,0 +1,41 @@
package com.kupreychik;

Choose a reason for hiding this comment

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

com.kupreychik))))))))))))

Choose a reason for hiding this comment

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

исправить название пакета


@UtilityClass
public class FileConsts {
public static final String FILE_STUDENTS_GROUP1 = "C:/Java/zulu_project3/группа 1.txt";

Choose a reason for hiding this comment

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

а почему тут полный путь? на моем пк такого пути не будет и программа упадет

Comment on lines 18 to 19
log.debug("Проверка пройдена. Дата подходит");
LocalDate.parse(model.getBirthday(), DateTimeFormatter.ofPattern(DATE_FORMAT));

Choose a reason for hiding this comment

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

будет прикольно, когда у тебя будет лог что проверка пройдена, а потом в методе parse выпадет исключение)))

Choose a reason for hiding this comment

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

вводить данные из json было бы гораздо проще, чем парсить такой текст и превращать это в объекты

private final List<Teacher> teachers = new ArrayList<>();


public TeacherServiceFromMyComputer() {

Choose a reason for hiding this comment

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

я понимаю, что эти методы для тестирования локального кода. но:

  • читай данный из json, а не текста, ты же сама видишь, как проще вызвать objectMapper.read, чем писать такую сложную логику
  • если этот класс тебе нужен для локально разработки, пометь его с помощью javaDoc

public class StudentServiceFromMyComputer {
private final List<Student> list= new ArrayList<>();

public StudentServiceFromMyComputer(String fileName) {

Choose a reason for hiding this comment

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

Тоже самое, описал в класс TeacherServiceFromMyPC

Choose a reason for hiding this comment

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

мы вроде обсуждали что это будет REST приложение, которое возращает json и просто можно будет из Postman'a дергать это.

Comment on lines 88 to 92
@SneakyThrows
private void sendOk(HttpServletRequest req, HttpServletResponse resp, Object object) {
req.setAttribute("object", object);
getServletContext().getRequestDispatcher("/students.jsp").forward(req, resp);
}

Choose a reason for hiding this comment

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

тут кажется хотели делать REST

@antonkupreychik
Copy link

Про юнит тесты - начинай с малого, берешь самый маленький метод и пытаешься его протестировать на каких-то вводных данных. Начни с этого класс - StudentRepository он самый простой у тебя для тестирования, то есть ты тестируешь как сохраняются, удаляются и получаются студенты

Copy link

@antonkupreychik antonkupreychik left a comment

Choose a reason for hiding this comment

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

Все окей, нужно больше тестов

@@ -0,0 +1,41 @@
package com.kupreychik;

Choose a reason for hiding this comment

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

исправить название пакета

Comment on lines 23 to 26
public GroupController(GroupService groupService, JsonParseService jsonParseService) {
this.groupService = groupService;
this.jsonParseService = jsonParseService;
}

Choose a reason for hiding this comment

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

есть у ломбока такая аннотация, как @RequiredArgConstrcutor - почитай про нее

Comment on lines 48 to 52
if (url.contains("number")) {
Long number = Long.parseLong(url.substring((url.indexOf("=") + 1)));
responseAsString = groupService.getGroupByNumber(number);
} else if (url.contains("surname")) {
String paramSurname = url.substring((url.indexOf("=") + 1));

Choose a reason for hiding this comment

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

number surname = - все выноси в константы

}
}
}
return null;

Choose a reason for hiding this comment

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

Возвращай лучше Optional вместо null

return jsonParseService.writeToJson(new ErrorResponse("Cannot found timetable for groupNumber"));
}
}
@SneakyThrows

Choose a reason for hiding this comment

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

а зачем тебе @SneakyThrows если у тебя уже есть обработка исключения

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.

2 participants