Skip to content

Nathan sharmarke - #1

Open
nharren wants to merge 17 commits into
codefellows-seattle-301d21:masterfrom
sharmarkei:nathan-sharmarke
Open

nharren wants to merge 17 commits into
codefellows-seattle-301d21:masterfrom
sharmarkei:nathan-sharmarke

Conversation

@nharren

@nharren nharren commented Jun 24, 2017

Copy link
Copy Markdown

Single-line Summary

Today, Sharmarke and Nathan paired together. It took about 2:20

Reflect and summarize on your process for each TODO item :

  1. First, we started on setting up author stats template.
  2. Next, we wrapped articleview and article in IFFY.
  3. Next, we added our filter/reduce/map functions.
  4. Finally we added the stretch goal with an article count.

Checklist (before submitting, fill in each set of square brackets with an 'x')

  • We have titled the Pull Request similar to our branch name (ex: 'brian-rick').
  • This PR includes commits from both myself and my partner; e.g. We followed good pair programming practices by switching driver/navigator roles.
  • There is no extraneous, unrelated code included in this PR.
  • We have summarized our TODO: process above.

Comment thread starter-code/public/scripts/article.js Outdated
Article.all.push(new Article(ele));
});
*/
rows.map(ele => Article.all.push(new Article(ele)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remember .map returns a new array, so instead of doing Article.all.push() you could just assign Article.all = rows.map(.....)

Comment thread starter-code/public/scripts/article.js Outdated
// DONE: Chain together a `map` and a `reduce` call to get a rough count of all words in all articles.

Article.numWordsAll = () => {
return Article.all.map(a => a.body.length).reduce((acc, cur) => acc + cur);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What is a.body.length going to return you? Here a represents each article. And a.body is the body of the article and a.body.length is going to return you the length of the body. But you actually want the length of the article by words. SO you'r missing a step -----> split!

Comment thread starter-code/public/scripts/article.js Outdated

// DONE: Chain together a `map` and a `reduce` call to produce an array of unique author names. You will
// probably need to use the optional accumulator argument in your reduce call.
Article.allAuthors = () => Article.all.map(a => a.author).reduce((acc, cur) => acc.includes(cur) ? acc : acc.concat([cur]), []);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Note: Instead of using acc.concat, you could just use push like so acc.push(cur)

Comment thread starter-code/public/scripts/article.js Outdated
author: author,
articleCount: Article.all.filter(a => a.author === author).length,
wordCount: Article.all.filter(a => a.author === author)
.map(a => a.body.length)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here too! Split it!

@cathy810218

Copy link
Copy Markdown

Missing package.json file.

@nharren

nharren commented Jun 27, 2017

Copy link
Copy Markdown
Author

package.json should not be missing. You can find it here on our repo https://github.com/sharmarkei/10-functional_programming/blob/nathan-sharmarke/starter-code/package.json

@nharren nharren closed this Jun 27, 2017
@nharren nharren reopened this Jun 27, 2017
@cathy810218

cathy810218 commented Jun 27, 2017 •

Copy link
Copy Markdown

Hmm weird, for some reason I didn't see it. My bad!
Oh I guess it was because you guys didn't add fs in you dependencies, so it didn't show in Files Changed.

Comment thread starter-code/public/scripts/article.js Outdated

Article.numWordsAll = () => {
return Article.all.map(a => a.body.length).reduce((acc, cur) => acc + cur);
return Article.all.map(a => a.body.split().length).reduce((acc, cur) => acc + cur);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

split(' ')

@cathy810218

Copy link
Copy Markdown

Nice :) 👍

@nharren

nharren commented Jun 27, 2017

Copy link
Copy Markdown
Author

It was working before even without installing fs. IIRC, fs is a default node package, right?

@cathy810218

cathy810218 commented Jun 27, 2017 •

Copy link
Copy Markdown

You're absolutely right. We don't need to include fs in package.json. So yea you can totally remove that :)

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