patternjavaspringMinor
Super sophisticated game information web application
Viewed 0 times
applicationsupersophisticatedgamewebinformation
Problem
Considering I've got an exam on Spring tomorrow, I figured you could review a small example application I whipped together as preparation (and as such, I'd prefer the emphasis to be on Spring-related concepts).
Its functionality is straightforward: you start with a list of 3 games and you can edit or view its details, both which will lead to a different page. The former will allow you to update its description while also providing validation on the field.
Any suggestions?
`public class Game {
private int i
Its functionality is straightforward: you start with a list of 3 games and you can edit or view its details, both which will lead to a different page. The former will allow you to update its description while also providing validation on the field.
Any suggestions?
GameController@Controller
@RequestMapping("/games")
public class GameController {
@Autowired
private IGameRepository gameRepository;
@Autowired
private GameValidator gameValidator;
@RequestMapping(value = "/all", method = RequestMethod.GET)
public String index(Model model){
model.addAttribute("games", gameRepository.getGames());
return "index";
}
@RequestMapping(value = "/details/{id}", method = RequestMethod.GET)
public String details(Model model, @PathVariable("id") int id){
Game game = gameRepository.getGameById(id);
model.addAttribute("currentGame", game);
return "details";
}
@RequestMapping(value = "/edit/{id}", method = RequestMethod.GET)
public String edit(Model model, @PathVariable("id") int id){
Game game = gameRepository.getGameById(id);
model.addAttribute("edit_game_form", game);
return "edit";
}
@RequestMapping(value = "/edit/{id}", method = RequestMethod.POST)
public ModelAndView update(@ModelAttribute("edit_game_form") @Valid Game game, BindingResult bindingResult, Model model){
gameRepository.updateGame(game);
gameValidator.validate(game, bindingResult);
if(bindingResult.hasErrors()){
return new ModelAndView("edit", "edit_game_form", game);
}
model.addAttribute("games", gameRepository.getGames());
return new ModelAndView("index");
}
}
Game`public class Game {
private int i
Solution
GameIn the code you shared, you never use the setters of the
Game class. How about making its fields final and removing the setters? Immutable things are simple, robust, and great. It's best to make everything immutable as much as you can.Also, do you really need the empty constructor?
GameRepositoryIn
getGameById and updateGame you iterate over a list of games. How about using a Map of id -> Game instead? It would seem a lot more natural, and at the same time more efficient.Formatting
Overall the code is well readable. The one thing where you violate the standard is spaces around braces and parentheses:
for(Game game : games){
if(game.getId() == id){The standard (Eclipse does it with
Control-Shift-f) would be:for (Game game : games) {
if (game.getId() == id) {Code Snippets
for(Game game : games){
if(game.getId() == id){for (Game game : games) {
if (game.getId() == id) {Context
StackExchange Code Review Q#61117, answer score: 7
Revisions (0)
No revisions yet.