Skip to content

Lab due1124#7

Open
bhamrickatmills wants to merge 12 commits into
MillsMCS:mockingfrom
bhamrickatmills:lab-due1124
Open

Lab due1124#7
bhamrickatmills wants to merge 12 commits into
MillsMCS:mockingfrom
bhamrickatmills:lab-due1124

Conversation

@bhamrickatmills
Copy link
Copy Markdown

No description provided.

@ellenspertus
Copy link
Copy Markdown
Contributor

Oops, I don't think you meant to check in the log files, such as hs_err_pid44175.log.

@bhamrickatmills
Copy link
Copy Markdown
Author

bhamrickatmills commented Nov 28, 2020 via email

@ellenspertus ellenspertus changed the base branch from complete to mocking December 7, 2020 20:26
Copy link
Copy Markdown
Contributor

@ellenspertus ellenspertus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since they've been committed, you need to git rm the log files.

Good job on the first part. Slack me when you've done the second part.

Comment thread .gitignore Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/main/java/edu/mills/cs180a/wordui/model/SampleData.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
Comment thread src/test/java/edu/mills/cs180a/wordui/model/SampleDataTest.java Outdated
*
* @param wordApi API that returns a word's information
* @param wordsApi API that returns a random word
* @param backingList A list of listeners for changes in objects
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameter descriptions should start with lower-case letters (except for capitalized acronyms). Don't include type information; it's shown automatically. Just refer to the ObservableList<WordRecord> as a list.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewritten! Thank you!

}

/**
* Fill backing list with Word of the Day and predefined sample words.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good.

/**
* Fill backing list with Word of the Day and predefined sample words.
*
* @param backingList a list of Word Records
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per above comment, "the list" or "the list of words".

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amended to match above.

@SuppressWarnings("unchecked")
@Test
void getWord_True_CorrectWordReturned() {
assertEquals(SAMPLE_WORD_STRING, SampleData.getWordOfTheDay(mockWordsApi).getWord());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Store SampleData.getWordOfTheDay(mockWordsApi) in a local variable.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stored in a variable called "word".

void getWord_True_CorrectWordReturned() {
assertEquals(SAMPLE_WORD_STRING, SampleData.getWordOfTheDay(mockWordsApi).getWord());
assertEquals(SAMPLE_WORD_DEF, ((Map<Object, Object>) SampleData
.getWordOfTheDay(mockWordsApi).getDefinitions().get(0)).get("text").toString());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have an assertion about the size of the list before calling .get() on it.

Copy link
Copy Markdown
Author

@bhamrickatmills bhamrickatmills Dec 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be a JUnit assertion, or just an assertion? I decided to go with JUnit for the added information.

assertEquals(SAMPLE_WORD_STRING, SampleData.getWordOfTheDay(mockWordsApi).getWord());
assertEquals(SAMPLE_WORD_DEF, ((Map<Object, Object>) SampleData
.getWordOfTheDay(mockWordsApi).getDefinitions().get(0)).get("text").toString());
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also test the frequency.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't call the static method. Use a local variable to reference the WordRecord added to the list, and call getFrequency(), getWord(), and getDefinition() on that.

@ellenspertus
Copy link
Copy Markdown
Contributor

ellenspertus commented Dec 22, 2020 via email

* @param wordApi API that returns a word's information
* @param wordsApi API that returns a random word
* @param backingList A list of listeners for changes in objects
* @param wordApi returns a word's information
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your previous descriptions of wordApi and wordsApi were better.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted

assertEquals(SAMPLE_WORD_STRING, SampleData.getWordOfTheDay(mockWordsApi).getWord());
assertEquals(SAMPLE_WORD_DEF, ((Map<Object, Object>) SampleData
.getWordOfTheDay(mockWordsApi).getDefinitions().get(0)).get("text").toString());
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't call the static method. Use a local variable to reference the WordRecord added to the list, and call getFrequency(), getWord(), and getDefinition() on that.

ObservableList<WordRecord> backingList = FXCollections.observableArrayList();
assertEquals(0, backingList.size());
SampleData.addWordOfTheDay(mockWordApi, mockWordsApi, backingList);
assertEquals(1, backingList.size());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, but it would be better to use a local variable to reference the WordRecord added to the list, and call getFrequency(), getWord(), and getDefinition() on that.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted to local variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants