Spring Boot RESTful service

I have developed this restful service using spring boot. Please look at the code if there is something to change to be more consistently.

Resource

```
@RestController
@RequestMapping(value = "/api/cars")
public class MuscleCarResource {

@Autowired
private MuscleCarService muscleCarService;

@RequestMapping(value = "/get-car/{id}", method = RequestMethod.GET)
public ResponseEntity getMuscleCar(@PathVariable("id") int id) {

try {
MuscleCar muscleCar = muscleCarService.getCar(id);
if (muscleCar != null) {
return ResponseEntity.status(HttpStatus.OK).body(muscleCar);
} else {
return ResponseEntity.status(HttpStatus.NOT_FOUND).build();
}

} catch (Exception e) {
return ResponseEntity.status(HttpStatus.BAD_REQUEST).build();
}

}

@RequestMapping(value = "/delete-car/{id}", method = RequestMethod.DELETE)
public ResponseEntity deleteMuscleCar(@PathVariable("id") int id) {

try {
muscleCarService.removeCarFromList(id);
return (ResponseEntity) ResponseEntity.status(HttpStatus.OK);
} catch (Exception e) {
return (ResponseEntity) ResponseEntity.status(HttpStatus.BAD_REQUEST);
}

}

@RequestMapping(value = "/add-car", method = RequestMethod.POST)
public ResponseEntity addCarToList( @RequestBody MuscleCar muscleCar) {

try {
muscleCarService.addCarToList(muscleCar);
return ResponseEntity.status(HttpStatus.OK).build();
} catch (Exception e) {
e.printStackTrace();
return ResponseEntity.status(HttpStatus.BAD_REQUEST).build();
}
}

@RequestMapping(value = "/cars", method = RequestMethod.GET)
public ResponseEntity>> listAllCars() {

try {
List> result = muscleCarService.listAllCars();
return ResponseEntity.status(HttpStatus.OK).body(result);
} catc

Should be changed:

• Request mappings are not that RESTful due to the verbs:

• POST /api/cars/add-car => POST /api/cars

• GET /api/cars/cars => GET /api/cars

• GET /api/cars/get-car/{id} => GET /api/cars/{id}

• PUT /api/cars/update-car/{id} => PUT /api/cars/{id}

• DELETE /api/cars/delete-car/{id} => DELETE /api/cars/{id}

• POST can return HTTP 201 Created.

• deleteMuscleCar causes unchecked warnings due to the generic casts. You can suppress the warnings for the whole method with @SupressWarnings("unchecked") since it's short (however, I would extract local variables and suppress warnings for the variables narrowing down the scope for the @SuppressWarnings annotation).

• There are methods named remove. They must be named delete. See the difference here.

• You have missed ex.printStackTrace(); for several controller methods.

• HTTP 400 Bad Request may be not a subject to be logged (it's a too common error).

• POST /api/cars/add-car only parameter is annotated with @RequestBody and the body is required by default, so the service "create" method may have the null check removed.

• POST /api/cars/add-car should return a created entity.

• GET /api/cars/cars cannot return HTTP 400 Bad Request.

• PUT /api/cars/update-car/{id} should return a modified entity.

• DELETE /api/cars/delete-car/{id} should return HTTP 204 No Content.

• You can reduce the amount of code in your controllers when checking for errors using @ControllerAdvice-annotated classes. This allows you to define default controller response status and not create response entities yourself delegating this job to Spring Framework. @ResponseStatus is used to set default response statuses.

• Having no results must return an empty collection, never null. Note how queryForList works.

• Technically, IDs can be negative.

• MuscleCarDaoImpl dependencies can be changed: you can omit DataSource and configure it in a more appropriate place like configuration files.

Depends on code conventions, style and preferences:

• Java annotations value can be omitted if it's a single annotation argument. For example, @RequestMapping("/api/cars")

• Java static imports can improve readability. For example, return status(OK).body(result);

• Controller methods parameters tend to have annotations making lines of code too lengthy. You can put a single annotated method parameter per line.

• Your controller is a CRUD-controller: Create, Read, Update and Delete. Maybe it's better to rearrange the controller methods as addCarToList, listAllCars, getMuscleCar, updateCar, deleteMuscleCar?

• You can make the controller dependencies final and create an @AutoWired constructor.

• I would not use public modifier as much as possible. Spring Framework is smart enough to find not public components.

• MuscleCarService interface might be introduced in order to have loose coupling.

• Collection.size() > 0 can be replaced with !Collection.isEmpty().

• Throwing business exceptions could be more preferable rather than returning special values. Say, NoSuchElementException may be thrown when no entity found by ID, and then caught in the controller exceptions advice.

• final Object[] args = new Object[] { foo, bar, baz } can be replaced with shorter final Object[] args = { foo, bar, baz }.

• queryForObject mapper and args can be swapped in favor of varargs methods (variadic parameters). update is also variadic.

• Note that MuscleCarRowMapper can be a singleton: it has no state thus should not be instantiated in every update. Also its constructor can be private (we instantiate it itself, we know better) and the static accessor can even weaken the return type (what if you need to replace/wrap an instance of another concrete type?). Nevertheless, this mapper is too specific (a very special mapper for 4 columns) and can be even made static+private in the DAO class (can be anonymous class, but I think that it can be a method letting Java 8 compiler to align the mapper method with the RowMapper interface with a method reference).

My personal preferences:

• I would name interface with starting with I. Say, I would rename Foo and FooImpl to IFoo and Foo respectively. In my opinion, Impl looks worse than I.

• I would prefer UPPER_CASE for SQL statements.

Representation layer

`@RestController
@RequestMapping("/api/cars")
final class MuscleCarController {

private final IMuscleCarService muscleCarService;

@Autowired
MuscleCarController(
final IMuscleCarService muscleCarService
) {
this.muscleCarService = muscleCarService;
}

@RequestMapping(method = POST)
@ResponseStatus(CREATED)
MuscleCar create(
@RequestBody final MuscleCar muscleCar
) {
return muscleCarService.create(muscleCar);
}

@RequestMapping(method = GET)
@ResponseStatus(OK)
List> get() {
return muscle

StackExchange Code Review Q#158891, answer score: 9