DEV Community

loading...

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

Jack Harner πŸš€
I'm Jack! I'm a Freelance Developer. I have experience with WordPress, Shopify, BigCommerce, React, Linux Admin, & More! Ask Me Anything!!
・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")
);

Enter fullscreen mode Exit fullscreen mode

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!

Discussion (7)

Collapse
unbrandedtech profile image
James Kenaley

Shouldn't the cutoff check use the isBefore function?

if (now > todaysCutoff)
if (now.isBefore(todaysCutoff))
Collapse
jackharner profile image
Jack Harner πŸš€ Author

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

Collapse
dmfay profile image
Dian Fay

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

Collapse
jackharner profile image
Jack Harner πŸš€ Author

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.

Collapse
dmfay profile image
Dian Fay

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.

Collapse
gypsydave5 profile image
David Wickes

THIS THIS THIS 100 X THIS

Collapse
daviddalbusco profile image
David Dal Busco • Edited

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