Skip to content

Sanjana's Implementation of Identifying the Most Common Word in a Paragraph#5

Open
sanja-jonna wants to merge 5 commits intomainfrom
sanjana
Open

Sanjana's Implementation of Identifying the Most Common Word in a Paragraph#5
sanja-jonna wants to merge 5 commits intomainfrom
sanjana

Conversation

@sanja-jonna
Copy link

No description provided.

@sanja-jonna sanja-jonna requested a review from danego September 20, 2020 22:21

var bannedArray = [];
bannedArray.push(banned);
wordsInPargrphArr = wordsInPargrphArr.filter(word => word != banned);
Copy link

Choose a reason for hiding this comment

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

Always use !== instead of != (and same for ===) in Javascript, unless you're specifically looking to coerce values. I know the udemy course has more info somewhere in the first half


getDictWithFrequencies(paragraphArr, bannedWordsArr) {
var frequencyDict = new Object();
if(key in frequencyDict) {
Copy link

Choose a reason for hiding this comment

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

I'd add a space between 'if' and first (
Same for 'for' loops (line 21 looks good)

Copy link

Choose a reason for hiding this comment

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

Also, haven't seen (propertyName in objectName) used before--that's cool. Most of the time I tend to see objectName.hasOwnProperty(propName), which makes sure that the property is not on the object's prototype

getDictWithFrequencies(paragraphArr, bannedWordsArr) {
var frequencyDict = new Object();
if(key in frequencyDict) {
value = frequencyDict[key];
Copy link

Choose a reason for hiding this comment

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

I'm pretty sure you should have declare 'value' with var or let here, unlike Object.entries which provides [key, value].
Without a var, it becomes an implicit global level variable

Copy link

Choose a reason for hiding this comment

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

pretty sure just in case I'm missing something like Object.entries

var wordsOfHighFrequency = []

for(const [key, value] of Object.entries(frequencyDict)) {
if(value == max) {
Copy link

Choose a reason for hiding this comment

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

===

Copy link

@danego danego left a comment

Choose a reason for hiding this comment

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

Nice Javascript code--it looks good! I thought you named your variables and functions well and the overall look matched what I have in mind for JS.

Three quick notes:

  1. One thing you could change would be adding more functions. For example, you could add a very simple function to prepare the paragraph (~lines 61) or adding function calls w/in getWordsWithHighestFreq(). Tbh, I tend to code more like your code here and don't like to abstract away too much, but Sirrele is always pushing for more functions (and single purpose ones). Robert's branch had a lot of well-designated functions, for reference
  2. The task.md said the answer would be unique, so no need to make wordsOfHighFrequency an array, but good job nonetheless.
  3. I like the node bonus stuff you added!

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