10
\$\begingroup\$

In my BlogList.vue component I made a search input field:

<input type="text" placeholder="Enter key word  ..." v-model="search">

And a computed property:

computed: {
    getfilteredData() {
     return this.blogs.filter(blog =>
        blog.name.toLowerCase().includes(
          this.search.toLowerCase()
        ) ||
        blog.category.toLowerCase().includes(
          this.search.toLowerCase()
        )
      ) 
    }
  },

The user can search on multiple values, blog.name and blog.category, which updates the list accordingly.

But can I write the above JavaScript (computed property) in a cleaner way?

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Jun 15 at 17:04
  • \$\begingroup\$ How many blog posts do you search? Hundreds at most wouldn't be a problem but it shouldn't iterate over thousands and thousands of posts. In this case, you'd need another data type (e.g. database, Trie, ...) \$\endgroup\$ – Eric Duminil Jun 17 at 13:03
7
\$\begingroup\$

It is really hard to say, but at least for me, your code is fine. It seems readable and has no issues. Perhaps if you want to change the style and make it more "canonical", you could write instead:

computed: {
    getfilteredData() {
      return this.blogs.filter(blog =>
        blog.name.toLowerCase().includes(this.search.toLowerCase()) ||
          blog.category.toLowerCase().includes(this.search.toLowerCase())
      )
    }
  },

But for me (and I believe, most readers of this post) your code is just fine.

Note: for "canonical" I mean fewer lines of code and more compactness. Clearly, it should be done rationally, and you should not write a 200 characters line. The point is write less and allow readability.

Edit

As Sᴀᴍ Onᴇᴌᴀ answered, it could be substantially optimized if you do:

    getfilteredData() {
      const lowerCaseSearch = this.search.toLowerCase();
      return this.blogs.filter(blog =>
        blog.name.toLowerCase().includes(lowerCaseSearch) ||
          blog.category.toLowerCase().includes(lowerCaseSearch)
      )
    }

because you are creating the lowercase string of your search field only once and using it twice instead of creating it twice and use it twice.

| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ thanks totally clear! \$\endgroup\$ – meez Jun 15 at 16:52
  • 2
    \$\begingroup\$ If you write the || in front of the following line it gets even clearer, because you are less likely to overlook it. Also, if you have multiple (read many) lines, you can immediately see, if you screwed up an operator. Much easier at least, than if it was at the end of the line. \$\endgroup\$ – infinitezero Jun 16 at 17:51
12
\$\begingroup\$

One improvement would be factoring out the repeated call to this.search.toLowerCase() and storing the result in a block-scoped constant so it:

  1. doesn't need to be calculated on every call to .includes() for each iteration of filter()
  2. can decrease the length of each line where that value is used

That may be a small optimization but it is in line with the D.R.Y. principle

getfilteredData() {
  const lowerTerm = this.search.toLowerCase()
  return this.blogs.filter(blog =>
    blog.name.toLowerCase().includes(lowerTerm) ||
    blog.category.toLowerCase().includes(lowerTerm)
  ) 
},
| improve this answer | |
\$\endgroup\$
6
\$\begingroup\$

Another way is to create an array from name and category and combine some method with include:

const result = blogs.filter(blog =>
    [blog.name, blog.category].some(s => s.toLowerCase().includes(searchRow.toLowerCase()))
);

It would be very convenient to scale your method by adding into array just necessary property. Let me show an example:

let blogs = [
    { name: "name 1", category: "category 1"},
    { name: "name 2", category: "category 2"},
    { name: "name 3", category: "category 3"},
];

let searchRow = "name 1";
let result = blogs.filter(blog =>
    [blog.name, blog.category].some(s => s.toLowerCase().includes(searchRow.toLowerCase()))
);
console.log(result);

| improve this answer | |
\$\endgroup\$
5
\$\begingroup\$

It doesn't look like this has been suggested yet but you can use object destructuring so you don't have to repeat blog a couple of times.

computed: {
  getfilteredData() {
    const term = this.search.toLowerCase()
    
    return this.blogs.filter(({ name, category }) => {
      return name.toLowerCase().includes(term) || category.toLowerCase().includes(term)
    })
  }
}
| improve this answer | |
\$\endgroup\$
  • 1
    \$\begingroup\$ You're missing two .toLowerCase() calls though \$\endgroup\$ – Bergi Jun 16 at 18:24
3
\$\begingroup\$

As already mentioned, the code is ok, but if you want to make it cleaner by removing code duplication, you can introduce local variables and helper functions:

getfilteredData() {
  const term = this.search.toLowerCase();
  const isMatched = str => str.toLowerCase().includes(term);
  return this.blogs.filter(blog => isMatched(blog.name) || isMatched(blog.category));
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.