loading...

Can I Get Some Feedback? (Moment.js in particular)

jackharner profile image Jack Harner πŸš€ ・1 min read

I'm setting up this thing for work where we would display on our website if a customers order is going to be shipped out today or on the next available business day. Our cut off is 12 Noon Mountain Time, and we only ship out M-F.

Here's the relevant code:

var now = moment().tz("America/Denver");
var availableDays = [1, 2, 3, 4, 5];
var todaysCutoff = now.clone().hour(12);
var expectedProcessing = "";

// If today is after 12 Noon, Expected Processing goes to tomorrow.
if (now > todaysCutoff) {
    expectedProcessing = now.clone().add(1, 'day');
} else {
    expectedProcessing = now.clone();
};

// If Expected Processing is not during the week, set to Monday of next week
if (!availableDays.includes(expectedProcessing.day())) {
    expectedProcessing =  expectedProcessing.add(1, 'week').day(1);
};


$("#processing").html("Expected Ship Date: " + 
    expectedProcessing.format("MMMM Do YYYY hh:mm")
);

Basically, if it's after Noon, set the Ship Date to the next day. If the Ship Date is a weekend, push it out to the next Monday.

As far as I can tell it works, was just curious about the feedback from someone more knowledgable than me on Moment.js/moment-timezone.

Thanks!

Posted on by:

jackharner profile

Jack Harner πŸš€

@jackharner

I'm Jack! I'm a Front-End Engineer. I have experience with WordPress, React, Linux Admin, & More! Let's Chat!

Discussion

markdown guide
 

Shouldn't the cutoff check use the isBefore function?

if (now > todaysCutoff)
if (now.isBefore(todaysCutoff))
 

The way I had it written it needed isAfter instead, but this is the exact kind of feedback I was looking for. Thank you.

 

Instead of going by feel or trial and error, why not write some unit tests and prove it does what you think it does?

 

Having not done any testing, I'm not even sure how I would start testing this. Any resource recommendations on testing this sort of simple function? Everything I've seen so far more relates to testing pieces inside complex apps.

 

Check out Mocha's in-browser test harness. It has you set up a page which pulls in the test framework Mocha and Chai (a library of assertions like equal, isTrue, lengthOf, and so on). You'll need to put your logic in a function so you can invoke it from your testcases as well as the "real" call sites, and ideally it should produce easily verifiable outputs, decoupling it from the display code; or alternatively you could look into mocking libraries like Sinon.

 
 

Instead of format I would maybe rather use the calendar view?

I personally rather like to read "Your expected ship date is tomorrow at 9:09 AM" than "Your expected ship date is 2019 11th May 09:09 AM"

moment(expectedProcessing).calendar(); 

About the calculation there is a moment().weekday(); method. Not sure if fits your use case but maybe you don't have to create an array of available dates.

Also I would maybe not initializeexpectedProcessing to a string (expectedProcessing = "") as it will be soon afterwards be transformed in a date.

About cloning dates, I never used that method and I ask my self if it isn't a bit less performant than just creating a new Date() specially as you are interested on knowing "today".

Finally, does the comparison now > todaysCutoff always work? isn't that comparing two objects references? When I compare dates I always compare their time so I'm sure the comparison will work, something like the following or as James Kenaley suggested with isBefore

if (now.toDate().getTime() > todaysCutoff.toDate().getTime()) {
    // something
}

Final thought, if I may of course, Moment.js is really handy and the calendar view and i18n is really nice but, that being said, if you only use Moment for that purpose your clients will have to fetch a lot of Javascript code just for that purpose. In such cases you might want to have a look to other lighter date manipulation library like date-fns or others