Ocena małego projektu aplikacji webowej napisanej w Java + Spring

pytanie zadane 17 marca 2018 w Nasze projekty przez xenonso Mądrala (5,160 p.)

Witam
Proszę o ocenę kodu mojego małego projektu forum internetowego. Projekt był napisany od zera jako projekt na informatykę. Po czasie widzę, że spora część rozwiązań była niedopracowana i wiem jak je poprawić. Projekt nie jest w pełni skończony, ale uznałem, że tylko REST i nie chcę się pchać w TML ¯\_(ツ)_/¯. Wybaczcie, że jest też on po angielsku, ale chciałem sobie przypomnieć trochę języka.

Użyte technologie: Java, Spring, Spring Security, Thymeleaf, Hibernate, MySQL oraz Mockito do kilku testów.

Oczywiście wiem, że frontend leży, ale nie jest to moje zainteresowanie i wolałbym o ocenę wyłącznie backendu.

Link do GitHub

1	komentarz 17 marca 2018 przez Aisekai Nałogowiec (42,190 p.) Po co Ci dao jak masz repositories?

komentarz 17 marca 2018 przez xenonso Mądrala (5,160 p.)

Wiem, że można to zrobić bez tego, ale widziałem takie rozwiązanie w książce "Spring w Akcji. Wydanie 4". Z tego co zrozumiałem pozwala to na uniknięcie wiązania z konkretną implementacją, a dostęp do danych jest niezależny od technologii.

komentarz 17 marca 2018 przez xenonso Mądrala (5,160 p.)

Dobra, widzę teraz, że to nie powinno się nazywać DAO, bo nie spełnia to raczej jego założeń.

1	komentarz 17 marca 2018 przez Tomasz90 Nałogowiec (25,140 p.) PIerwsze co mi się w rzuciło to te konstruktory w klasie User. Zastąp je builderem. Przy okazji dodaj sobie Lomboka do projektu.

1	komentarz 17 marca 2018 przez Mariusz08 Maniak (62,300 p.) @xenonso, https://github.com/xenonso/ITForum/blob/master/src/main/java/itforum/config/DatabaseConnectionConfig.java#L32 Dobrze, że nie było faktycznych danych do bazy, ale na wszelki wypadek usuń to.

komentarz 17 marca 2018 przez xenonso Mądrala (5,160 p.)

@Mariusz08

Myślę, że nie ma potrzeby usuwać. Oczywiście wiem, że root-root to najgorsze połączenie, ale nie miałem zamiaru tego uruchamiać publicznie, więc tak zostawiłem.

komentarz 17 marca 2018 przez xenonso Mądrala (5,160 p.)

@Tomasz90

To jest jeden z tych mankamentów, o których wspomniałem. Dodanie buildera jest moim kolejnym krokiem. Niestety trochę za późno się zorientowałem jaki potworek z tego powstał :/

1 odpowiedź

odpowiedź 18 marca 2018 przez ShiroUmizake Nałogowiec (46,300 p.)
wybrane 18 marca 2018 przez xenonso

Najlepsza

Controler nie powinen posiadać logiki:

@RequestMapping("/login")
public class LoginController {
	
	@RequestMapping(method = GET)
	public String login(
			Model model,
			@RequestParam(value = "logout",	required = false) String logout,
			@RequestParam(value = "failure",	required = false) String login
		){
		if(logout != null){
			model.addAttribute("logoutMessage", "You have been successfully logged out");
		}
		
		if(login != null){
			model.addAttribute("failureMessage", "Login failed, check your nick and password");
		}
		
		return "loginPage";
	}
}

Powinen tym sie zająć serwis podajać mu args.

Repository które nie jest repository, bardziej mu podpada po serwis gdyż sporo logiki tu jest:

if (result.hasErrors()) {
			return "registrationPage";
		}else if(!userRepository.isEmailAvailable(user.getEmail())){
			result.addError(new ObjectError("email","An account already exists for this email"));
			return "registrationPage";
		}else if(!userRepository.isNickAvailable(user.getNick())){
			result.addError(new ObjectError("nick","An account already exists for this nick"));
			return "registrationPage";
		}else{
			user = setStartValuesForUser(user);
			user.setPassword(passwordEncoder.encode(user.getPassword()));
			userRepository.saveUser(user);

			return "redirect:/profile/"+user.getNick();
		}
	}

Plus wydaje mi się że lambdą byłoby znacznie mniej kodu.

Dobry przykład kontrolera.

	@RequestMapping(method = GET)
	public String adminControlPanel(){
		return "adminPage";
	}

Prosty łatwy a właściwie uniwesalny, łatwy do przenoszenia w inny projekt.

Spring ma w domyśle swoje repository.

Query query = entityManager.createNativeQuery(
				  "SELECT fp.id, fp.title, fp.content, fp.date FROM forumpost AS fp "
				+ "INNER JOIN forumcategory AS fc "
				+ "ON fc.id = fp.idCategory "
				+ "WHERE fc.id=?1");
		query.setParameter(1, id);

można łatwiej używając queryParams. Unoikaj native jak najbardziej możliwe.

private LinkedList<ForumPost> mapToPostList(List<Object[]> posts){

Dlaczego na Object?

public interface UserRepository{
	 User getUserById(Long id);
	 User getUserByNick(String nick);
	 User getUserByEmail(String email);
	 List<User> getAllActiveUsers();
	 boolean isNickAvailable(String nick);
	 boolean isEmailAvailable(String email);
	 User saveUser(User user);
	 User update(User user);
	 void disableUser(User user);
	 void disableUserByNick(String nick);
	 Long countUsers();
}

Zapoznaj się z generetykami w JAVA.

Projekcik całkiem spoko, szkoda że nie masz LIVE version.

komentarz 18 marca 2018 przez xenonso Mądrala (5,160 p.)

Dzięki za rady.

Object dlatego, że native zwraca tego typu tablicę, a tak łatwiej mi się zmieniało to na obiekty. Co do generyków, to nie pomyślałem, żeby ich tu użyć, zapamiętam na przyszłość.