REST API to list books, where null means "everything"

My company has a REST API, and one design issue that has come up a few times is that we use null to indicate "get everything".

For example /api/books?genre=romance will get all romance books, but /api/books will return all books from all genres.

The current technique used in the code is something like this

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {

    List bookDtos = new ArrayList<>();
    List allBooks = getAllBooks();

    for (Book book : allBooks) {
       // This if condition is what I take issue with
       if (genre == null || book.genre.equals(genre)) {
           bookDtos.add(book.convertToDto());
       }
    }
    return bookDtos;

}

I think it is bad practice to use null here to convey the meaning of get everything. Is this correct?

My current solution is to distinguish between these two cases at the very top level like so:

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {

    if (genre == null) {
        return getAllBooks();
    } else {
        return getBooks(genre);
    }
}

And then a few supporting functions that funnel all of the logic into the same place by massaging the inputs a little bit.

// This is the core code, that takes a list of genres and returns all books from all those genres.
public Collection getBooks(List genre) {
    // And here is the code that will actually get the books from the database.
    // ...
    // ...
}

// This is a supporting function, that finds all genres and funnels that into the above function.
public Collection getAllBooks() {
    List allGenres = getAllGenres()
    return getBooks(allGenres);
}

// This is a supporting function that builds a list from a single genre, to again funnel into the 1st function.
public Collection getBooks(String genre) {
    List genres = new ArrayList<>();
    genres.add(yearCode);
    return getBooks(genres);
}

The idea here is to have a single place where the work actually takes

I personally think it's fine, but it helps to turn your thinking around from a "selector" to a "filter". Then, the default is to return all books, and you selectively filter down the set you're going to eventually respond with.

(Note I haven't written Java in a long time, so grain of salt here,) I might specify a BookFilter object which can incrementally have fields set from the parameters, and then have BookDTO.filterBooks. Then, your request method becomes:

public ResponseEntity getBooks(@RequestParam(required = false) String genre) {
    BookFilter filter = new BookFilter();
    if (genre != null) {
      filter.setGenre(genre);
    }

    return filterBooks(filter);
}

I know it's not directly applicable (your DTO probably doesn't work this way, based on what you described above), but I think what you're doing with the null check in your controller is pretty much the same thing with the tools you've got.

You might consider trying to refactor your DTO so it can work with filters however, because eventually you may want ?genre=scifi&published=1970+&publisher=penguin etc. etc., at which point it'll be a lot easier to manage a single filter object than a bunch of explicit queries.

StackExchange Code Review Q#160381, answer score: 22