We have a lot of temporary variables. It's just not clear. shouldFlag takes in an array of IDs and these are selected IDs. We're essentially going to use that to examine this.messages and examine the flagged property on those and decide whether we're going to flag or unflag them.
What that might look like is something like this where you can select multiple messages and flag and unflag. I've set up some example messages at the bottom of the file here, just some dummy messages, along with this function that's going to help us confirm that we are maintaining our existing behavior.
This is the existing behavior. If all of them are flagged, unflag them, either select the messages...If we have messages 124 and 126 selected, those are both flagged. We're going to go ahead and unflag. That should be false, and so on and so on for each of our scenarios.
The first portion that I want to clean up is how we're building up this boolean value that we're ultimately returning. This is overly complicated and rather than setting a temporary variable to false and assuming don't flag and conditionally inserting that if none of them are flagged or if some of them are flagged here and setting to true, we can go ahead and just ask that question and assert this at the bottom and return that value directly.
We can just drop several lines to begin. The largest area of opportunity I see is that we're doing a lot of looping and iterating here and we're starting out an empty array and we're iterating through using the eye and counter method this.messages.
One of the biggest things I'm looking for when I jump into refactoring some existing code is a lot of this iteration and looping, especially when we're starting with an empty array and building it up conditionally, pushing onto it.
We're going to make use of the ES6 arrow functions here, tidy this up. What this is saying is this is our parameter and we're going to have a statement here that's going to return true or false and we can replace this with message.id. That's from our parameter we're passing in there.
This function is going to return true if this message's id is contained in the selected ids array. Now we can replace this, filter does return an array, so we can go ahead and just take that here. I like to put these on new lines, just for readability. Rip this out. Run our test and we're still passing. We can more appropriately name messages in question to selected messages for clarity.
Let's see what we can do about cleaning up this next set of iteration over selected messages. We know that this is an array, so let's make use of those built in methods again. Selected messages for each is going to go through each message in the set, take that as a parameter, and what our function is going to be doing is just this.
We're just going to replace that for I iteration with the built in for each. It looks like we need to make sure we're, instead of selected messages I, we just have message.
If we examine the logic closely, really the only question we need to be asking is are any of the messages not flagged. We can prove that by taking this out and just asking the single question if the unflagged count is not equal to zero when these iterators are building up.
We can go ahead and drop this flag count, we're no longer using that. Right now, what we're going to be asking if not message flagged we're going to increment this. If we run that test, we're still passing just the same.
At this point we can see that really we're iterating through this selected messages array and we're seeing if any of these messages are not flagged. We can go ahead and use the built in some method, which is going to take a test function also. Our test is that if the message is not flagged...
What some does is it takes a test function and if any of the items in the array pass that function, it's going to return true. It's basically asking, "Do any of them pass this test?" Now, we can go ahead and remove this, remove this, and we no longer have a need for this unflagged count variable.
We can just return the result of that some operation directly and our tests are still passing. If you remember, we began with this very lengthy implementation here and by making use of some of those built in array methods and just clarifying what is our objective, we really clean that up quite a bit.