Conversation
| for (Meal meal : meals) { | ||
| if (isResultOfSearch(meal, params)) { | ||
| searchedMeal.add(meal); | ||
| int frequency = getFrequency(meal, params); |
There was a problem hiding this comment.
Add a test that checks whether an entry with more matches is returned at the beginning of the results list, favoured over other entries.
| } | ||
|
|
||
| private Boolean isResultOfSearch(Meal meal, List<String> params) { | ||
| private int getFrequency(Meal meal, List<String> params) { |
There was a problem hiding this comment.
Yet another suggestion to the getFrequency method.
Given that it's become fairly complicated, it could use some unit tests. We can test it through the MealServlet (and we should), but that's a bit cumbersome.
Another option would be to move the method to the Meal class. If we made it a public method of Meal, we could:
- distribute the code more evenly among files (it's usually a good practice to have many, relatively small files, rather than one massive file that implements everything)
- make the
Mealclass more useful (anyone, not just MealServet, would be able to calculate it's word frequency) - and, most importantly, we could unit test the
getFrequencymethod in unit tests of the Meal class. Writing unit tests forMealwill be much simpler than writing tests forMealServlet, because the class is way simper.
| * @param params list of strings with keywords, that describe search request. | ||
| * @return amount of occurrences of keywords from params in this meal. | ||
| */ | ||
| public int getFrequency(List<String> params) { |
There was a problem hiding this comment.
I'd suggest adding a unit test for this method in a MealTest.java file.
A test that would check the exact number returned by this method for some instances.
| for (String ingredient : ingredients) { | ||
| if (ingredient.contains(param)) { | ||
| frequency++; | ||
| continue; |
There was a problem hiding this comment.
This continue will skip the remaining part of the inner loop and go to the next iteration. However, there's nothing happening after the continue, therefore this keyword does nothing (FWIU). This continue doesn't really do anything.
As a result, if a meal has 3 ingredients:
- chocolate flakes
- chocolate milk
- chocolate whatever
then as long as the word "chocolate" is not in the title / description / type, returned frequency will be 3. However, if the title does include the word "chocolate", we'll return early with frequency == 1. It looks like a tiny bug.
I believe you may have meant to break the outer loop instead. I think the simplest solution is to extract the second for loop to a method, e.g. bool doesKeywordAppearInIngredients(String keyword). Then you could drop all those continue keywords from this method and just have a single if.
4 participants
Now list of results displays in descending order, first recipe has the biggest number of keywords from request, next has less or equal, and so on.