Skip to main content
Code Review

Return to Answer

edited body
Source Link
sidgate
sidgate
  • 141
  • 5
  1. You do not need ping within your controller. Check out Spring boot actuator
  2. Each method having duplicated error handling. You can move it to @ControllerAdvice
  3. Method names like playerPOST, playerDELETE aren't helpful. Consider this as a normal class and have normal method names like createPlayer, deletePlayer
  4. You need not define produces attribute for each mapping, RestController defaults to JSON
  5. Returning ok("") doesn't make sense. Should be either returning the player object, or use ResponseEntity.created() (201)
  6. Why are you autowiring HttpServletRequest context?
  7. Should youuse AllArgsConstructor - constructor based autowiring
  1. You do not need ping within your controller. Check out Spring boot actuator
  2. Each method having duplicated error handling. You can move it to @ControllerAdvice
  3. Method names like playerPOST, playerDELETE aren't helpful. Consider this as a normal class and have normal method names like createPlayer, deletePlayer
  4. You need not define produces attribute for each mapping, RestController defaults to JSON
  5. Returning ok("") doesn't make sense. Should be either returning the player object, or use ResponseEntity.created() (201)
  6. Why are you autowiring HttpServletRequest context?
  7. Should you AllArgsConstructor - constructor based autowiring
  1. You do not need ping within your controller. Check out Spring boot actuator
  2. Each method having duplicated error handling. You can move it to @ControllerAdvice
  3. Method names like playerPOST, playerDELETE aren't helpful. Consider this as a normal class and have normal method names like createPlayer, deletePlayer
  4. You need not define produces attribute for each mapping, RestController defaults to JSON
  5. Returning ok("") doesn't make sense. Should be either returning the player object, or use ResponseEntity.created() (201)
  6. Why are you autowiring HttpServletRequest context?
  7. Should use AllArgsConstructor - constructor based autowiring
Source Link
sidgate
sidgate
  • 141
  • 5

  1. You do not need ping within your controller. Check out Spring boot actuator
  2. Each method having duplicated error handling. You can move it to @ControllerAdvice
  3. Method names like playerPOST, playerDELETE aren't helpful. Consider this as a normal class and have normal method names like createPlayer, deletePlayer
  4. You need not define produces attribute for each mapping, RestController defaults to JSON
  5. Returning ok("") doesn't make sense. Should be either returning the player object, or use ResponseEntity.created() (201)
  6. Why are you autowiring HttpServletRequest context?
  7. Should you AllArgsConstructor - constructor based autowiring