3
\$\begingroup\$

I'm making a news aggregator using newsAPI, and have everything working - to begin with. But was wondering if the code I use to filter through an object can be made more efficient - meaning fewer lines of code.

So far, it only returns what I need from the raw JSON file, then filters it using a for loop to ignore objects with blank values.

Any help would be great.

   if (!error && response.statusCode == 200) {

        let data = JSON.parse(body);

        let articles = data.articles.map(article => {
            return {
                "title": article.title,
                "date": article.publishedAt,
                "image": article.urlToImage,
                "description": article.description,
                "link": article.url
            }
        });

        let filtered = []
        for (let i = 0; i < articles.length; i++) {
            if (articles[i].title !== null &&
                articles[i].date !== null &&
                articles[i].image !== null &&
                articles[i].description !== null &&
                articles[i].link !== null) {
                filtered.push(articles[i])
            }
        }

        console.log(filtered)

        res.render("index", { filtered: filtered });

   }
\$\endgroup\$
3
  • \$\begingroup\$ Too lazy to write an answer. Array to hold property names, filter then map. array.reduce to transform each object. Chain them to avoid intermediates and you are done. const keys = [["title", "title"], ["publishedAt", "date"], ["urlToImage", "image"], ["description", "description"], ["url", "link"]]; res.render("index", { filtered: JSON.parse(body).articles .filter(article => keys.some(key => article[key[0]] !== null)) .map(article => keys.reduce((art, k) => (art[k[1]] = article[k[0]], art), {})) }); BTW your code is missing a closing } \$\endgroup\$ Commented Aug 3, 2019 at 15:45
  • 2
    \$\begingroup\$ Looks like closing brace for the outer if is missing. That begs the question, is there other code missing? \$\endgroup\$ Commented Aug 3, 2019 at 18:51
  • \$\begingroup\$ Your code could be so much cleaner if you used the same property names throughout your code. Is that possible? \$\endgroup\$ Commented Aug 5, 2019 at 15:10

2 Answers 2

Reset to default
2
\$\begingroup\$
  • let -> const
  • Use the Array methods (filter, every)
  • Use object destructuring
  • Use object short-hand
  • Use a method for the null check so you don't have to write it for every property
if (!error && response.statusCode == 200) {
  const data = JSON.parse(body);

  const filtered = data.articles
    .filter(({ title, publishedAt, urlToImage, description, url }) =>
      [title, publishedAt, urlToImage, description, url].every(
        prop => prop !== null
      )
    )
    .map(
      ({
        title,
        publishedAt: date,
        urlToImage: image,
        description,
        url: link
      }) => ({
        title,
        date,
        image,
        description,
        link
      })
    );

  res.render("index", { filtered });
}

Update: filtered first for lower memory consumption as by @FreezePhoenix suggestion.

\$\endgroup\$
7
  • 1
    \$\begingroup\$ You seem to miss that the source properties and the rendered properties do not have the same name \$\endgroup\$ Commented Aug 5, 2019 at 15:08
  • \$\begingroup\$ @konijn Actually still had it wrong 😅 Now it's fixed. \$\endgroup\$ Commented Aug 5, 2019 at 15:18
  • \$\begingroup\$ Hm... this creates a 2 new objects for each one being filtered... perhaps there is a better way? \$\endgroup\$ Commented Aug 5, 2019 at 15:37
  • 2
    \$\begingroup\$ @FreezePhoenix Not quite sure what you are getting at. Do you mean to lower the space complexity? \$\endgroup\$ Commented Aug 5, 2019 at 15:48
  • 2
    \$\begingroup\$ @Mohrn Your new edit in an attempt to incorporate my suggestion errors :) because none of the variables are defined. \$\endgroup\$ Commented Aug 5, 2019 at 16:05
1
\$\begingroup\$

Revising @Mohrn's answer:

  • Avoid creating new objects if possible.
  • As mentioned by @radarbob, you are excluding the closing if bracket.

As such, I recommend a few changes be made:

if (!error && response.statusCode == 200) {
  const data = JSON.parse(body);

  const filtered = data.articles
    .map(
      ({
        title,
        publishedAt: date,
        urlToImage: image,
        description,
        url: link
      }) => (
        ([title, date, image, description, link].every(_ => _ !== null) ?
            { title, date, image, description, link } :
            null
      )
    )).filter(_ => _ !== null);

  res.render("index", { filtered });
}

This does result in fewer objects being created per iteration.

\$\endgroup\$
2
  • \$\begingroup\$ You're returning objects with the shape { title, publishedAt, utlToImage, description, url } rather than { title, date, image, description, link }. The difference between our solutions is that you lack the mapping of property names. \$\endgroup\$ Commented Aug 5, 2019 at 15:51
  • \$\begingroup\$ @Mohrn Edited, now the difference is that it doesn't create a second object for ones that don't need to be made. Instead of making the object and then getting values, we're getting values and then making the object if needed. \$\endgroup\$ Commented Aug 5, 2019 at 15:54

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.