Skip to content

Richelle (water)#38

Open
r-spiel wants to merge 8 commits into
Ada-C14:masterfrom
r-spiel:master
Open

Richelle (water)#38
r-spiel wants to merge 8 commits into
Ada-C14:masterfrom
r-spiel:master

Conversation

@r-spiel

@r-spiel r-spiel commented Dec 14, 2020

Copy link
Copy Markdown

Assignment Submission: JS Adagrams

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

Prompt Response
What patterns were you able to use from your Ruby knowledge to apply to JavaScript? Basic looping and if statements still work the same, but being able to nest methods is different.
Did you need to use different strategies to find information online about JavaScript than you would for Ruby? I looked at a lot of mdn examples, but still used a lot of similar google searching it just was clear that javascript does not have a lot of built in methods, so I'd have to code with the more basic building blocks.
What was something you needed to do independent research on for this project? What did you learn? Using the .find method for js.
What was a challenge you were able to overcome on this assignment? Debugging was very different and I still need to figure out how to use the debugger tool. But using print statements worked well enough for me.
What has been interesting and positive about learning a new programming language? It has been very positive for me to see that the basic buiding blocks are mostly the same - if statements loops.
What is something to focus on your learnings in JavaScript in the next week? Debugging.

@beccaelenzil beccaelenzil left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

JS Adagrams

Major Learning Goals/Code Review

Criteria yes/no, and optionally any details/lines of code to reference
Correctly uses variables, and only uses const and let variables. The program prefers const variables. The program never uses var. ✔️
Practices best-practices in JavaScript syntax. There are semi-colons at the end of most lines that need semi-colons. Variables and functions are named with camelCase. ✔️
Correctly creates and calls functions within an object with proper syntax (parameters, return statements, etc.) ✔️
Uses correct syntax for conditional logic and iteration ✔️
Practices git with at least 3 small commits and meaningful commit messages ✔️
Utilizes unit tests to verify code; tests can run using the command $ npm test test/adagrams.test.js and we see test successes and/or failures ✔️

Functional Requirements

Functional Requirement yes/no
For the drawLetters function, there is an appropriate data structure to store the letter distribution. (You are more likely to draw an 'E' than an 'X'.) ✔️
Utilizes unit tests to verify code; all tests for drawLetters and usesAvailableLetters pass ✔️
Utilizes unit tests to verify code; all tests for scoreWord pass ✔️
Utilizes unit tests to verify code; all tests for highestScoreFrom pass ✔️

Overall Feedback

Great work on this project! In this project you’ve taken some interesting logic and worked it into JavaScript syntax. Your code is clear and readable. I've left a few in-line comments on ways you might consider refactoring. Please let me know if you have any questions. Keep up the hard work!

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 5+ in Code Review && 3+ in Functional Requirements ✔️

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Perfect Indentation
Elegant/Clever
Descriptive/Readable
Concise
Logical/Organized

Comment thread src/adagrams.js
const Adagrams = {
drawLetters() {
// Implement this method for wave 1
const letterQuantity = {"A": 9, "B": 2, "C": 2, "D": 4, "E": 12, "F": 2, "G": 3, "H": 2, "I": 9, "J": 1, "K": 1, "L": 4, "M": 2, "N": 6, "O": 8, "P": 2, "Q": 1, "R": 6, "S": 4, "T": 2, "U": 4, "V": 2, "W": 2, "X": 1, "Y": 2, "Z": 1};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider moving each letter to its own line to increase readability and changeability.

Comment thread src/adagrams.js
const times = x => f => {
if (x > 0) {
f();
times (x - 1) (f);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Minor note: remove space between function and arguments to increase readability.

Suggested change
times (x - 1) (f);
times(x - 1) (f);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fun use of recursion!

Comment thread src/adagrams.js
scoreWord(word) {
let score = 0;

for (const letter of word) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Consider how storing each letter score in an object and using that value to update the score could DRY up this code.

Comment thread src/adagrams.js
Comment on lines +79 to +103
let highestScorer = scores[0];
for (let item of scores) {
if (item['score'] > highestScorer['score']) {
highestScorer = item;
}
}

let ties = []
for (let item of scores) {
if (item['score'] === highestScorer['score']) {
ties.push(item);
}
}

let winner = ties.find(word => word['word'].length == 10 ) // finds the fir st word with ten letters

if (winner) {
return winner;
} else { // find the word with min length
winner = ties[0]
for (let item of ties) {
if (item['word'].length < winner.word.length) {
winner = item
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There are 3 loops that might run in this function. Consider how you could refactor to include the logic of tie-breaking as you go through the original loop.

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