Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

add date/diff, date/startOf, date/isSame, time/convert, date/totalDaysInYear #82

Merged
merged 5 commits into from Apr 25, 2013

Conversation

Projects
None yet
4 participants
Owner

millermedeiros commented Apr 5, 2013

see #80, #28

PS: while coding the diff method I also found out that moment.js implementation is wrong for months/years so we should report the problem and fix to them. It's way harder than it seems.

@millermedeiros millermedeiros commented on an outdated diff Apr 5, 2013

tests/spec/date/spec-diff.js
+
+
+ it('should handle year switch', function () {
+ var d3 = new Date(2012, 11, 15);
+ var d4 = new Date(2013, 0, 15);
+ expect( diff(d3, d4, 'month') ).toEqual( 1 );
+ });
+
+
+ it('should handle months with fewer days', function () {
+ var d3 = new Date(2012, 10, 27);
+ var d4 = new Date(2013, 1, 14);
+ // yes, it isn't 2.5 since partial month should start at day 27
+ // of previous month (so it should add 3 days since November
+ // has 30 days)
+ expect( diff(d3, d4, 'month') ).toEqual( 2.607142857142857 );
@millermedeiros

millermedeiros Apr 5, 2013

Owner

I'm still not sure if this is the correct behavior.. it feels right since February is the base date and for us what really matters is when the month will overlap to March, but at the same time the fractional value would be different if we used the average days of the start and end months... - date manipulation is hard.

@millermedeiros millermedeiros commented on an outdated diff Apr 5, 2013

src/date/diff.js
+
+ if (startDay !== endDay) {
+ // calculate fractional based on total days of last month
+ // (since it will only "loop" to a full month after it
+ // reaches the end of the month)
+ var startTotalDays = totalDaysInMonth(start);
+ var baseTotalDays = totalDaysInMonth(end);
+ var baseDay = (startDay > endDay)? startDay - startTotalDays : startDay;
+ var daysElapsed = endDay - baseDay;
+
+ // use average of days per month since it should only count
+ // as a full month after reaching the next month start day
+ // eg: Jan 28 - Feb 26 (29 days elapsed but not a full
+ // month)
+ if (daysElapsed > baseTotalDays) {
+ baseTotalDays = (startTotalDays + baseTotalDays) / 2;
@millermedeiros

millermedeiros Apr 5, 2013

Owner

this is clearly wrong.. it shouldn't use the average.. it should detect how many days from startDay until the end of the month (1st day of next month). If the baseTotalDays > startDay it should consider that next month will be at end.setDate(startDay)...

Owner

millermedeiros commented Apr 8, 2013

daylight time savings will mess up with diff:

// daylight time savings
var a = new Date('Wed Feb 12 2014 00:00:00 GMT-0200 (BRST)');
// not anymore...
var b = new Date('Sun Feb 16 2014 00:00:00 GMT-0300 (BRT)');
diff(a, b, 'days'); // 4.041666666666667

unsure about what is the correct behavior

Owner

conradz commented Apr 8, 2013

Wouldn't date/diff use GMT/UTC time? That should remove the daylight savings time problems.

Owner

millermedeiros commented Apr 12, 2013

I also added a new method time/convert which can be useful in many cases and did some refactoring to clean the code.

PS: still missing documentation for all methods.

@millermedeiros millermedeiros referenced this pull request in moment/moment Apr 12, 2013

Merged

Fix month and year diffs #571

@millermedeiros millermedeiros commented on an outdated diff Apr 12, 2013

src/date/diff.js
@@ -0,0 +1,127 @@
+define(['./totalDaysInMonth', '../time/convert'], function(totalDaysInMonth, convert){
+
+ /**
+ * calculate the difference between dates (range)
+ */
+ function diff(start, end, unitName){
+ var output = arithmeticDiff(start, end, unitName);
+
+ if (output && unitName) {
+ if (unitName === 'month' || unitName === 'year') {
+ var monthsDiff = getMonthsDiff(start, end);
+
+ if (unitName === 'year') {
+ output = monthsDiff / 12;
@millermedeiros

millermedeiros Apr 12, 2013

Owner

this is an approximation! if the diff is 15 days on a 31 day month the result will be different than 15 days on a 28 day month.

15 / 31 / 12 = 0.04032258064516129
15 / 28 / 12 = 0.044642857142857144

the margin of error should be between 0.0002 and 0.007 which for me seems reasonable given the amount of work required to get the accurate result when the dates spans through multiple years and some of them are leap years.

@millermedeiros

millermedeiros Apr 17, 2013

Owner

I ended up changing the behavior to be accurate. A correct system is better than a simple system.

@millermedeiros millermedeiros commented on the diff Apr 12, 2013

src/date/diff.js
+ end = toUtc(end);
+ }
+ return Math.abs(start - end);
+ }
+
+
+ function toUtc(d){
+ // we ignore timezone differences on purpose because of daylight
+ // savings time, otherwise it would return fractional days/weeks even
+ // if a full day elapsed. eg:
+ // Wed Feb 12 2014 00:00:00 GMT-0200 (BRST)
+ // Sun Feb 16 2014 00:00:00 GMT-0300 (BRT)
+ // diff should be 4 days and not 4.041666666666667
+ return Date.UTC(d.getUTCFullYear(), d.getUTCMonth(), d.getUTCDate(),
+ d.getHours(), d.getMinutes(), d.getSeconds(),
+ d.getMilliseconds());
@millermedeiros

millermedeiros Apr 12, 2013

Owner

I tried to calculate the timezone difference but I couldn't get IE7 to create date objects on a different timezone and it was going to be very error prone - new Date('Wed Feb 12 2014 00:00:00 GMT-0200 (BRST)') returns 11pm of Feb 11 on the user locale (UTC-0300 on my case).

I'm still not sure if that is the correct behavior (user might be comparing dates from a different timezone altogether - not related to DST) but it was the best solution that I could think of so far.

Since browsers create date objects on the user locale by default (and there is no way to set a custom one) I don't think it will cause problems.

Owner

millermedeiros commented Apr 16, 2013

now the partial years/months diff is accurate!

I will rebase the whole thing so the commit order makes sense (will squash some commits) and will also write docs for it.

this is what I call yak shaving...

Owner

millermedeiros commented Apr 17, 2013

rebased the commits and added documentation. I think it's good to go. Please review it when you get some time.

/cc @conradz, @leocavalcante

@millermedeiros millermedeiros added a commit that referenced this pull request Apr 25, 2013

@millermedeiros millermedeiros Merge pull request #82 from millermedeiros/wip-date_diff
add date/diff, date/startOf, date/isSame, time/convert, date/totalDaysInYear
013655c

@millermedeiros millermedeiros merged commit 013655c into mout:master Apr 25, 2013

1 check passed

default The Travis build passed
Details

@millermedeiros millermedeiros deleted the millermedeiros:wip-date_diff branch Apr 25, 2013

Can you give a example of that. I want to know how to use it.

rands0n commented Feb 1, 2017 edited

@DurgpalSingh you want a example of how to use diff date? Or something differen function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment