Weekday fixes #49
Weekday fixes #49
Conversation
fix for weekday calculation when first weekday is not 1=sunday (if set explicitly by "+ (void)mt_setFirstDayOfWeek:(NSUInteger)firstDay" or implicitly by locale)
It shouldn't return values more than 7
If today is Monday, and we say that the week starts from Friday, then `mt_startOfCurrentWeek` for today should return Friday from the past. It should never return a day after the receiver.
| * 26 27 28 29 30 31 | ||
| */ | ||
|
|
||
| NSDate *date = [_formatter dateFromString:@"08/20/2012 09:23am"]; | ||
|
|
||
| [NSDate mt_setFirstDayOfWeek:1]; | ||
| XCTAssertEqualObjects([[date mt_startOfCurrentWeek] mt_stringFromDateWithShortWeekdayTitle], @"Sun"); |
There was a problem hiding this comment.
Actually such tests doesn't seem to be right to me... If mt_startOfCurrentWeek returns a date from wrong week (i.e. from the next week), then the test still passes because it just compared just names of the days. Maybe we should remove them and to leave only updated tests...
|
@yas375 sure I'll have a look |
|
So I did some test, @yas375 not with your code but some more essential test with the Apple NSDate, NSCalendar and NSDateComponents. As you can see I use two different calendar settings one is manually the setting if I initialise the calendar directly with the English US local one is for German and Germany. I did this this way so the values are actually visible. Then I create components with year 2012 and weekOfYear 1. IMHO this should produce a date of the first day in the first week of year 2012. Can you explain this to me? |
|
@Alex9779 thanks a lot for the investigation! Unfortunately I still can't find some free time to deep into this. On the upcoming weekend will be busy at a hackathon so probably also won't have time, sorry :( If anyone else wants to join the discussion - welcome ;) |
|
Ok I did even some further test, this time I created a new OS X Cocoa Project with Xcode 6.1 in Swift and removed the main project and used only the tests. For example with firstWeekday = 1 and minimumDaysInWeek = 1 I expect the first weekday of the first week in year 2012 (that is year = 2012, weekOfYear = 1, weekday = 1) to be the 2012-01-01 but I get 2012-12-30 while for the first weekday of the second week (that is year = 2012, weekOfYear = 2, weekday = 1) which should be 2012-01-08 I get the correct result. With firstWeekday = 2 and minimumDaysInWeek = 4, thats the german or european standard, I expect the first weekday of the first week in year 2012 (that is year = 2012, weekOfYear = 1, weekday = 2) to be the 2012-01-01 but I get 2012-12-30 while for the first weekday of the second week (that is year = 2012, weekOfYear = 2, weekday = 2) which should be 2012-01-09 I get the correct result. So I opened a bug report at Apple, let see what they say... |
|
So what I wanna say with all this is that the framework of Apple may be buggy and the test of MTDates build on that system, initializing the calendar with several values, so the fix is only a fix for the values that MTDates presets for the test and my test also use that values. This does not mean that my fix really fixes week and weekday calculations on real application!!! IMHO this is not really correct, we should generate the expected dates as low level as possible... In my second test app I created them from components where I set year, month and day and used a paper calendar and really looked there with the set values of the NSCalendar in mind... |
In continuation of #37 and #48.
IMPORTANT note: the changes itself in MTDates seems to be reasonable to me, but when I brought this version of MTDates to the production app I'm working on I see lots of failing unit tests around date calculations :( I'm going to investigate further on another day when I have more time. To be honest I'm not sure I will be able to do this until next week.
If anyone has time to test this pull request and/or do code review - I'll very appreciate your support.
@Alex9779 could you please take a look at the fixes I done on top of yours? Thanks in advance!