DEV Community

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

Jack Harner πŸš€ on May 10, 2019

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 avail...
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 πŸš€

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 πŸš€

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