Skip to content

Commit

Permalink
Corrected formula for 29-feb :-)
Browse files Browse the repository at this point in the history
  • Loading branch information
@danielgindi
danielgindi committed Aug 9, 2016
1 parent 52fe95d commit 69777ad
Showing 1 changed file with 8 additions and 6 deletions.
14 changes: 8 additions & 6 deletions ChartsDemo/Classes/Formatters/DayAxisValueFormatter.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,18 @@ - (NSString *)stringForValue:(double)value

- (int)daysForMonth:(int)month year:(int)year
{
// month is 0-based

if (month == 1)
{
if (year == 2016 || year == 2020)
{
return 29;
}
else
int x400 = month % 400;
if (x400 < 0)
{
return 28;
x400 = -x400;
}
BOOL is29 = (month % 4) == 0 && x400 != 100 && x400 != 200 && x400 != 300;

return is29 ? 29 : 28;
}

if (month == 3 || month == 5 || month == 8 || month == 10)
Expand Down

5 comments on commit 69777ad

@ezamagni
Copy link
Contributor

@ezamagni ezamagni commented on 69777ad Aug 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is completely wrong and error prone!
You should definitely consider using the standard Foundation date arithmetic capabilities.
let calendar = NSCalendar.currentCalendar() let comps = NSDateComponents() comps.year = 2016 comps.month = 2 calendar.rangeOfUnit(.Day, inUnit: .Month, forDate: calendar.dateFromComponents(comps)!).length
You can copy and paste it in a playground to test it a little bit.

@danielgindi
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is actually 100% accurate.
It could be even shorter- but then with more division operations which would slow things down.

The point is not doing too much when drawing a frame. Using the built-in date arithmetic will definitely be slower. It requires more allocation, and many computations which we do not really need here.

If you want- you can set up a loop in the playground, between year -10000 and year 10000, or 10000000, and test my formula against the system-provided one. Remember to skip year 0 :)

@danielgindi
Copy link
Collaborator Author

@danielgindi danielgindi commented on 69777ad Aug 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember: The fact that you don't know how to do something simple because of "mistery" math, does not mean it's error prone. Somebody else did this for you in NSDate, and NSDate is great- but not for this purpose.

@ezamagni
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry m8, i was too hasty and harsh :(
Out of curiosity, i'll try to profile performances for both implementations as soon as have some spare time: date methods should be quite optimized in Foundation.
Keep up the good work, can't wait for version 3!

@danielgindi
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the other months it could be further optimized by having an indexed array by month.
But it's a thing of calculations only - it's about memory allocations which generally take a lot more time than a few math calculations. So the rule of thumb is "do not allocate in drawing code". This rule is cross-platform.

You can already try out version 3 - as it's in its final stages :-)
Just polishing stuff, mostly the demos.
@PhilJay and I have decided not to introduce any more breaking changes, leave those for the next version...

Please sign in to comment.