Getting products by category, price, etc

I have the following concerns:

-
Is Set a good choice for getting products? What are drawbacks of Set for this purpose?

-
Is it better to do something like common.retainAll() or to get products through a request with WHERE to the database. For now I do three requests in: getProductsByCategory, getProductsByManufacturer, getProductsByPrice. So maybe it would be better to make just one?

@RequestMapping("/{category}/{price}")
public String filterProducts(@PathVariable("category") String productCategory,
        @MatrixVariable(pathVar = "price") Map> priceParams,
        @RequestParam("manufacturer") String manufacturer, Model model) {

    Set productsByCategory = service.getProductsByCategory(productCategory);
    Set productsByManufacturer = service.getProductsByManufacturer(manufacturer);

    BigDecimal low;
    BigDecimal high;

    low = new BigDecimal(priceParams.get("low").get(0));
    System.out.println(low);
    high = new BigDecimal(priceParams.get("high").get(0));
    System.out.println(high);

    Set productsByPrice = service.getProductsByPrice(low, high);

    Set common = new HashSet(productsByCategory);

    common.retainAll(productsByManufacturer);
    common.retainAll(productsByPrice);

    model.addAttribute("products", common);

    return "products";

}

OK, so I assume this is the code from your @Controller, there's a @Service that it calls, and the Service further calls a @Repository (the DAO layer). Furthermore, I shall assume you're using the latest version of Java, i.e. Java 8.

Ultimately I find it all depends on the number of products you have. Given not too many, you could do all the filtering in Java. Given thousands, you'd better do it in the database.
One method in the database layer

In any case, one database call is definitely much better than three, so you could just create a generic enough database search method to which you can pass all the search parameters. When you create the method, you could also create a data transfer object named, say, SearchParameters that contains all the search parameters. That way your method signatures don't grow to ridiculous lengths even when you search with several parameters. The interface will also be easy to change, because in order to add a new search parameter the method signature won't have to change, just the data transfer object.
Personal experience

The way I've solved a similar issue, with just a couple of dozen products, was that my DAO layer had a trivial public List findAll() method with a @Cacheable annotation so that after the first call to the DB the further ones would only access a fast cache in the memory.

The Service would then have the methods you described: getProductsByCategory, getProductsByManufacturer and getProductsByPrice, but they'd all just call the findAll() method of the DAO (quickly getting the cached response) and they'd filter the results based on their parameters.

For example (assuming the Product class has a getManufacturer() method that returns a String:

public Set getProductsByManufacturer(final String manufacturer) {
    return dao.findAll().stream()
        .filter(product -> Objects.equals(product.getManufacturer(), manufacturer)
        .collect(Collectors.toSet());
}

And that's it using Java 8's streams. The filter only includes those products whose manufacturer matches the given parameter.
One method in the service layer

However, since you need an intersection of three sets, you might as well create one method to do all of your filtering. Either to the DAO layer like I proposed in the beginning, or do it in your service layer if the amount of data is not too big, or databases aren't your strong suit. ;)

So, your business code would boil down to something like this:
New SearchParameters class:

import java.util.Optional;

public class SearchParameters {
    // Here with just one search parameter for brevity, but you'll get the idea
    String manufacturer;

    public boolean manufacturerMatches(final Product product) {
        return Optional.ofNullable(manufacturer).map(m -> m.equals(product.getManufacturer())).orElse(false);
    }
}

A method in your Service class:

public Set getProducts(final SearchParameters params) {
    return dao.findAll().stream()
        .filter(params::manufacturerMatches)
        .collect(Collectors.toSet());
}

Then just add more of those matcher methods to the SearchParameters class and deploy them as filter lines in your Service -- you can chain filters, so it's easy and really concise.

In case you're not familiar with Java 8's streams, .filter(params::manufacturerMatches) is a short-hand notation for .filter(product -> params.manufacturerMatches(product)), which in turn compares all the products against the manufacturerMatches method and only allows a product to go through if the method returns true.
Answers

So, to answer your questions...

-
Sets are basically good for holding those Products of yours in case you are not interested in the order of the products. If that matters, you need to use some List instead. But that's not really what you asked or wanted to know, so:

The issue with Sets in your code is that you duplicate or triplicate a lot of data and operate on many more objects than you need to in order to achieve your objective. Comparing big product objects is also more expensive (in terms of CPU cycles) than simple Strings or other such parameters, which would be enough for you. You could have all of the objects (Products) in one collection and just filter that. Also when you compare your objects in Sets, you need to be sure your equals and hashCode methods for the Products are always up to date -- if you forget to update them when you add a new field, you will get wrong results.

-
Yes, it's best to do just one DB call -- and even that one call can sometimes be omitted by using a cache. You can achieve that "one call only" by creating a bit more complicated DB query to account for all the search parameters or by handling the searching (filtering) in Java. Which way should you do it, depends on the factors I've listed above, i.e. mainly about the number of products in your database and your personal preference. But I wouldn't really

StackExchange Code Review Q#106842, answer score: 4