A way to refactor this better?

JavaScript
1

Hello, I have a bit of code that I feel is inefficient as I am running a for loop twice. It’s functional but there has to be a better way to get this done. Is there a way I can do this in just one loop?

In the data object I have a created key with the value of Date.now() for when a post was created. I am storing created to the max variable if it’s the highest created number so that I can then add it to this.blog, which is another object because I want this.blog to only contain the newest blog post.

Any suggestions for cleaning this up is appreciated.

  mounted() {
    this.$binding("blogs", firestore.collection("blogs")).then(data => {
      let max = 0;
      for (let key in data) {
        if (data[key].created > max) {
          max = data[key].created;
        }
      }
      for (let key in data) {
        if (data[key].created === max) {
          this.blog = data[key];
        }
      }
    });
  }
2
  • What does the overall object look like, and what does the data structure it lives within look like?
  • Is this in React or similar, where you have a big data structure, and if so, what does that whole data structure look like?
3

I edited the original post to show you more of it. I am using vue, and a library to pull the data from firestore. I have a collection called blogs which contains the posted blogs. Each blog has author, content, title, created. All of which is a string but the created which is an int from Date.now().

4

Here you are doing two iterations for the same number of items, in the same object.

let max = 0;
// #1
for (let key in data) {
        if (data[key].created > max) {
          max = data[key].created;
        }
      }
// #2
for (let key in data) {
       if (data[key].created === max) {
          this.blog = data[key];
        }
      }

You could do this in one iteration by grabbing the object’s keys and using a forEach loop over them.

Accessing a property on an object is constant time, so it’s effectively a free operation.

let max = 0;
const keys = Object.keys(data)

keys.forEach(key => {
  if (data[key].created > max) max = data[key].created
  if (data[key].created === max) this.blog = data[key]
})
1 Like
5

Thanks! This is exactly what I was looking for.

6

What @JM-Mendez said is correct as things stand, but you shouldn’t be doing any iteration at all here, it makes no sense.

You have a database, and in that database is a collection. You want the most recent item (based on date) from that collection. This a database-level query. Downloading the entire dataset and iterating through it is very, very much the wrong thing to do: you should be asking the database for the single thing you want.

Because it’s a key:value data store that you’re using, this is much more difficult than it should be (the documents are not ordered in any way), but looking at the docs, the simplest way I can see to do this is:

  • Have a second collection (blog). This will only ever contain [max] one document.
  • Whenever a new item is added to the articles collection, if there is an item already in blog, delete it, then write a copy of the item to blog.
  • Render whatever is in blog to your component: it is guaranteed to be the one you want (if there are no articles, there will be nothing there, otherwise there will be the single document you want).

(sidenote: this quite neatly illustrates why a key:value database like Firestore is a really bad fit for this kind of data - with a relational database, you would have a created and/or an updated column, with a timestamp, and you just select the most recently created/updated row of data for your blog component - but that’s a different issue)

1 Like
7

I completely overlooked that this was a a firestore database. Sorry. I agree with @DanCouper and this should not be done in the ui.

The best way to do this with firestore is to use their query api with results in descending order, and limit the result to the latest blog
https://firebase.google.com/docs/firestore/query-data/order-limit-data

mounted() {
    this.$binding("blog", firestore.collection("blogs")
                                   .orderBy('created', 'desc')
                                   .limit(1)).then(blog => this.blog = blog)

I don’t agree that a relational db would be better. It all depends on how you plan to structure your data. Plus, relational databases have to run joins on all the data before serving it.

So while the querying may seem more natural, all the work is just abstracted away from the developer. But it still happens.

Which means you’d still need to do indexing for either. But firestore automatically indexes single-field keys. Meaning that if you keep a normalized structure, you’ll get that for free.

And it also allows you to make queries like:

citiesRef.where("country", "==", "USA").orderBy("population", "asc")

So with firestore you actually get the benefit of both types of databases.
https://firebase.google.com/docs/firestore/query-data/index-overview

2 Likes
8

Thanks for all the help guys. I am not that great with pulling data from firestore yet so I did not even know I could just pull one entry like that. This certainly helps a lot!