1

I'm trying to refactor some really bonkers and overcomplicated code that tries to do some sorting on an array of objects. The array looks like this:

[
  {
    "status": "CANCELLED",
    "createdDate": "2020-02-19T22:22:43Z",
    "dueDate": "2020-02-20T06:00:00Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-16T22:01:35Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-02-19T22:24:28Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-27T23:30:08Z"
  },
  {
    "status": "COMPLETE",
    "createdDate": "2019-08-27T04:53:46Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-15T08:39:24Z",
    "dueDate": "2020-01-16T08:38:00Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-16T22:02:43Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-16T04:48:56Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-03-31T04:07:17Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-27T23:27:25Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-03-12T03:28:24Z"
  },
  {
    "status": "COMPLETE",
    "createdDate": "2019-04-22T23:08:29Z",
    "dueDate": "2019-04-23T07:00:00Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-16T21:49:11Z",
    "dueDate": "2020-01-17T06:00:00Z"
  },
  {
    "status": "CANCELLED",
    "createdDate": "2019-04-22T23:13:22Z",
    "dueDate": "2019-04-23T07:00:00Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-21T23:36:43Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-06T02:51:48Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-16T03:46:44Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-16T03:46:11Z"
  },
  {
    "status": "OPEN",
    "createdDate": "2020-01-28T02:42:15Z"
  }
]

The conditions are as follows in order of priority:

  • The items that have a due date and a status of 'OPEN' come at the top and should be in ascending order of dueDate.
  • They should respect the following order of 'status': 'OPEN', 'COMPLETED', 'CANCELLED'
  • They should be ordered by 'createdDate' descending, so most recent items come at the top

I've been playing around with the Array.prototype.sort function learning how it works but at points not really knowing what I was doing. I was able however to meet the first and second conditions but not the third one.

The working code I have is:

this.items = this.items.sort((a, b) => {
    let rv = 0;
    if (b.status === 'OPEN' || a.status === 'CANCELLED') {
        rv = 1;
    }
    if (a.dueDate && a.status === 'OPEN') {
        rv = -1;
    }
    return rv;
});

But when I try to fit the third condition the other conditions are no longer respected. It would look something like:

if(new Date(a.createdDate).getTime() < new Date(b.createdDate).getTime()) {
  // return something
}

I'm thinking it may not be possible inside the same sort function and will have to split it into several, or maybe I need someone a lot wittier than I am.

Your help is appreciated

1
  • 2
    I think your main issue the more I look at this is that you're not early returning above, so you're potentially undoing any of the previous sorting. See my answer for a pattern that uses early returns.
    – Jacob
    Commented Mar 31, 2020 at 23:20

2 Answers 2

4

The way you sort by multiple conditions is you replace the 0 case of the first condition with the return values of the second condition, and you replace the 0 case of the second with the third. That logically follows if you think of this as "use this next condition if they're ranked the same with this prior condition."

const statusOrdering = {
  OPEN: 0,
  COMPLETED: 1,
  CANCELLED: 2
};

const dueDateOrdering = item => item.status === 'OPEN' && item.dueDate ? 0 : 1;

this.items = this.items.sort((a, b) => {
  // OPEN items with due dates come before those without.
  const dueDateDiff = dueDateOrdering(a) - dueDateOrdering(b);
  if (dueDateDiff) return dueDateDiff;

  // if a.status < b.status numerically, then the subtraction yields a negative
  // number. Or if the other way around, it's positive, so this orders by status
  // effectively.
  const statusDiff = statusOrdering[a.status] - statusOrdering[b.status];
  if (statusDiff) return statusDiff;

  // localeCompare returns a value compatible with sort methods.
  // Date strings of this format are already lexically orderable.
  return a.createdDate.localeCompare(b.createdDate);
});

A fun way to make this more readable would be to name your sort conditions:

const dueDateOrdering = item => item.status === 'OPEN' && item.dueDate ? 0 : 1;
const byOpenDueDate = (a, b) => dueDateOrdering(a) - dueDateOrdering(b);

const statusOrdering = {
  OPEN: 0,
  COMPLETED: 1,
  CANCELLED: 2
};
const byStatus = (a, b) => statusOrdering[a.status] - statusOrdering[b.status];

const byCreateDate = (a, b) => a.createdDate.localeCompare(b.createdDate);

this.items = this.items.sort((a, b) => 
  byOpenDueDate(a, b)
  || byStatus(a, b)
  || byCreateDate(a, b)
);
5
  • Thanks a lot for this! Been playing around a bit with it... It's not exactly right since it's bringing some of the CANCELLED items above the other status types, which should not be happening, will keep playing... nice head start and explanation though, thanks again.
    – andriusain
    Commented Apr 1, 2020 at 0:08
  • Actually my bad the first condition should be: The items that have a due date and are OPEN come at the top. Should be able to figure it out! Edited the original post to include this
    – andriusain
    Commented Apr 1, 2020 at 0:10
  • 1
    Nice, glad it's helpful. I've updated the answer to match your updates, I think.
    – Jacob
    Commented Apr 1, 2020 at 0:19
  • If I showed you the original code, you would feel the full extent of the triumph... :D
    – andriusain
    Commented Apr 1, 2020 at 1:00
  • It actually gets a bit more complicated than that, what if dueDateOrdering had to be in ascending order of 'dueDate'?
    – andriusain
    Commented Apr 1, 2020 at 2:13
0

So I had another requirement to the first condition stated in my original post (ticket updated) which is that: The items that have a due date and a status of 'OPEN' come at the top and should be in ascending order of dueDate.

I've been trying to make it work within the current logic but I wasn't able to, however I finally got it working by running another iteration of items.sort at the end of the code... This works and the whole thing is barely 20 lines of code which is much better than the original code I was refactoring (180 lines).

I'm pretty sure it can be simplified further but I'm really happy so far and this is what I have in the end:

const dueDateOrdering = item => item.status === "OPEN" && item.dueDate ? 0 : 1;
const byOpenDueDate = (a, b) => dueDateOrdering(a) - dueDateOrdering(b);

const statusOrdering = { OPEN: 0, COMPLETE: 1, CANCELLED: 2 };
const byStatus = (a, b) => statusOrdering[a.status] - statusOrdering[b.status];

const byDate = (a, b) => {
    if (a.dueDate && !b.dueDate) {
        return a.dueDate.localeCompare(b.dueDate);
    } else if (b.dueDate && !a.dueDate || b.dueDate && a.dueDate) {
        return b.dueDate.localeCompare(a.dueDate);
    }
    return a.createdDate.localeCompare(b.createdDate);
}

this.items = this.items.sort((a, b) => byOpenDueDate(a,b) || byStatus(a, b) || byDate(a, b));

this.items = this.items.sort((a,b) => {
    if (b.dueDate && a.dueDate && a.status === 'OPEN' && b.status === 'OPEN') {
        return a.dueDate.localeCompare(b.dueDate);
    }
    return 0;
});

Thanks again!

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.