addWordOfTheDay()#11
Conversation
|
@ellenspertus I'm not getting the option to add you as a reviewer here |
| * Adds the current word of the day to a given list of words. | ||
| * | ||
| * @param backingList a list to which the word of the day will be added | ||
| * @param wordsApi the API used to get today's word of the day |
There was a problem hiding this comment.
Excellent. Best in class.
There was a problem hiding this comment.
The parameter should be the API, per your javadoc, not the WordOfTheDay, per your implementation.
There was a problem hiding this comment.
I had originally passed the API as a parameter as stated in the javadoc, but it wasn't working correctly, and I missed updating the javadoc after changing the implementation. I'll update the javadoc
| @VisibleForTesting | ||
| protected static void addWordOfTheDay(List<WordRecord> backingList, WordOfTheDay word) { | ||
| List<Object> definitions = word.getDefinitions(); | ||
| if(definitions != null && !definitions.isEmpty()) { |
There was a problem hiding this comment.
Please put a space after reserved words (if).
|
|
||
| /*** | ||
| * Creates a list of standard sample words and their definitions along | ||
| * with the word of the day. Implements the wordnik API. |
There was a problem hiding this comment.
I'd drop the last sentence. This makes use of an API. It doesn't implement it.
| return wotd; | ||
| } | ||
|
|
||
| private static List<Object> makeDefinitions(Map<Object,Object> definition) { |
There was a problem hiding this comment.
Couldn't you replace this with a call to List.of()?
If you keep it, put a space after the comma (and other places you are missing a space after a comma).
| } | ||
|
|
||
| @Test | ||
| void getDefinition_True_mockedDefinition() { |
There was a problem hiding this comment.
I'd consider this a test of getWordOfTheDay() and would combine it with the preceding test.
getDefinitions() is part of the API, iirc, and not our job to test.
| when(SampleData.client.buildClient(WordApi.class)).thenReturn(mockWordApi); | ||
|
|
||
| LinkedList<WordRecord> backingList = new LinkedList<>(); | ||
| WordOfTheDay banana = SampleData.getWordOfTheDay(mockWordsApi); |
There was a problem hiding this comment.
Is this supposed to be here?
| SampleData.addWordOfTheDay(backingList, banana); | ||
|
|
||
| assertEquals("banana", backingList.get(0).getWord().toString()); | ||
| assertEquals(backingList.size(), 1); |
There was a problem hiding this comment.
Check the size first so you don't get an IndexOutOfBoundsError on the prior line.
Also, put the expected value first.
2 participants
Implements addWordOfTheDay(), fixes definition mocking which previously did not use a Map.
Also removed unnecessary tracked files. Sorry about that - now that I've added a bunch of extra files to multiple pull requests, I'll make a habit of double checking!