1
\$\begingroup\$

My code works as intended. CategoryController class has getAllCategories method, which tells ContentOrderMap class to map differently ordered categories to their respective OrderType enum.

The intention is whenever @RequestParam(defaultValue = "DEFAULT") OrderType categoriesOrderType changes from DEFAULT, to lets say, POST_COUNT_DESC, I would get differently ordered categories in my view.

My concern is if this is an efficient way of dealing with multiple ways of ordering my content. Previously I had a bunch of if statements in my controller and they would take care of getting and assigning information to model based on request param. This didn't seem very scalable since my controller method would get very big and would contain logic, which I think should not be there.

I'm having trouble correctly wording my problem and issues since I am new to this, if anything is not clear enough, I will gladly reword or explain.

Controller:

@Controller
@RequestMapping("/categories")
public class CategoryController {

private CategoryService categoryService;
private ContentOrderMap orderMap;

public CategoryController(CategoryService categoryService, ContentOrderMap orderMap) {
    this.categoryService = categoryService;
    this.orderMap = orderMap;
}

@GetMapping("/all")
    public String getAllCategories(@RequestParam(defaultValue = "0") int pageNumber, Model model, @RequestParam(defaultValue = "DEFAULT") OrderType categoriesOrderType) {
        orderMap.mapCategoriesToOrderType(pageNumber);
        Page<Category> categories = orderMap.categoriesByOrderType.get(categoriesOrderType);
        int pageCount = categoryService.getAllCategories(pageNumber).getTotalPages();
        model.addAttribute("orderType", categoriesOrderType);
        model.addAttribute("categories", categories.getContent());
        model.addAttribute("pageNumber", pageNumber);
        model.addAttribute("hasNextPage", categories.hasNext());
        model.addAttribute("pageCount", pageCount);
        return "categories";
    }
}

ContentOrderMap class

public Map<OrderType, Page<Category>> categoriesByOrderType = new HashMap<>();
public void mapCategoriesToOrderType(int pageNumber) {
        categoriesByOrderType.put(OrderType.DEFAULT, categoryService.getAllCategories(pageNumber));
        categoriesByOrderType.put(OrderType.POST_COUNT_DESC, categoryService.getAllCategoriesByPostCountDesc(pageNumber));
        categoriesByOrderType.put(OrderType.POST_COUNT_ASC, categoryService.getAllCategoriesByPostCountAsc(pageNumber));
        categoriesByOrderType.put(OrderType.FOLLOWER_COUNT_DESC, categoryService.getAllCategoriesByFollowersCountDesc(pageNumber));
        categoriesByOrderType.put(OrderType.FOLLOWER_COUNT_ASC, categoryService.getAllCategoriesByFollowersCountAsc(pageNumber));
        categoriesByOrderType.put(OrderType.POST_DATE_DESC, categoryService.getAllCategoriesByNewestPost(pageNumber));
        categoriesByOrderType.put(OrderType.POST_DATE_ASC, categoryService.getAllCategoriesByOldestPost(pageNumber));
    }

CategoryService

all of the methods seen above are more or less identical in service class, they only differ in their naming, so I am going to add only one of them for brevity's sake

public Page<Category> getAllCategoriesByPostCountDesc(int pageNumber) {
        Pageable pageable = PageRequest.of(pageNumber, 4);
        return categoryRepository.findAllOrderByPostCountDesc(pageable);
    }

In case its needed, I add an example of how categoryRepository looks

@Query(value = "SELECT c.* FROM category c LEFT JOIN review r ON c.id = r.category_id GROUP BY c.id ORDER BY COUNT(*) DESC",
            countQuery = "SELECT COUNT(*) FROM CATEGORY",
            nativeQuery = true)
    Page<Category> findAllOrderByPostCountDesc(Pageable pageable);
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Well, you found a workable solution.

One thing that strikes out at me is, that you expose the inner structure of your ContentOrderMap by accessing orderMap.categoriesByOrderType.get(categoriesOrderType) directly. The inner map categoriesByOrderType is an implementation detail, which you should not make known in the public interface. Rather give ContentOrderMap a direct method which returns the page (and while we are at it: change the name, as nobody should care about the class being or containing a map.)

A problem remains, which is: changes will not be local at one place. When you add a new OrderType, you have to extend the OrderType enum and the ContentOrderMap and the CategoryRepository.

Frankly, you'll not find a way to solve this problem in a 100% clean way, as it concerns the interface (i.e. the REST service), and the database, so if you assemble your actual definition of a type and the data storage effect it has in one place, you will have mixed responsibilities in that place.

However, if you think this mixing of responsibilities is bearable (everything is a tradeoff in software engineering ;-)) you might think about harnessing the power of the java enum, which can implement an interface. Something along the lines of:

public enum OrderType {
    DEFAULT("select c.* from whatever order by whatever"),
    POST_COUNT_DESC("select c.* from whatever order by something else"),
    ....;
    
    private final String query;
    
    private OrderType(String query) {
        this.query = query;
    }
    
    public String getQuery() {
        return query;
    }
}

This way, you only have to extend the enum, pass the constant on into your repository, and then do whatever you need to do with the contained query.

Another alternative is, using your container. I'm not using spring, but there must be something along the lines of a CDI Instance<>, which gives you a list of all implementations of a given interface.

In a CDI world, you could declare an interface:

public interface OrderHandler {
    public boolean handlesType(String orderType); // no enum, in the end REST sends you a string anyway
    public Page<Category> getResult(Pageable pageable);
}

Then write your different implementations of these interfaces, and get all of them via:

@Inject
Instance<OrderHandler> orderHandlers;

...

for (OrderHandler orderHandler : orderHandlers) {
    if (orderHandler.handlesType(myParameter)) {
        orderHandler.getResult(...);
    }
}

As I said, there must be something equivalent in spring.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.