yohang / CalendR

The missing PHP 5.3+ calendar management library.
http:/yohan.giarel.li/CalendR
MIT License
465 stars 65 forks source link

February ends in March #40

Closed garak closed 8 years ago

garak commented 9 years ago

This is a dump of February 2015:

Month {#2306 ▼
  -current: null
  #begin: DateTime {#2288 ▼
    +"date": "2015-02-01 00:00:00"
    +"timezone_type": 3
    +"timezone": "Europe/Rome"
  }
  #end: DateTime {#2295 ▼
    +"date": "2015-03-01 00:00:00"
    +"timezone_type": 3
    +"timezone": "Europe/Rome"
  }
  #firstWeekday: 1
}

I'd expect to see it ending 2015-02-28, not 2015-03-01.

timonf commented 9 years ago

This is not just on February... Every Period (month, day or week) should modified by minus one second.

Try out this one:

// __construct method on Month
$this->end->add(new \DateInterval('P1M'));
$this->end->modify('-1 second'); // add this line.

But this causes another issue: A period is by one second shorter. A week takes 7 days. With this fix, a week takes only 6 days, 23 hours, 59 minutes and 59 seconds.

jhice commented 9 years ago

Regarding the docs its seems to be normal : http://yohan.giarel.li/CalendR/periods.html

getEnd() : Returns the end \DateTime of the period. The end is excluded from the period

Does it explain your problem ? (does not explain how to handle it)

garak commented 9 years ago

Well, it's not so clear, but I see that it could be interpreted in a such way, so the end should be in effect March 1 instead of February 28? It doesn't make sense.

jhice commented 9 years ago

I just discovered this library because I'm interested in using it with Silex, so I looked into the issues to see what kind of problems are unsolved or raising.

Yohan should tell us why he did exclude the end like this, and not included it... Do we have to treat the information in the loop until this End date is reached ? I agree it's not very clear.

rodrigocalilo commented 8 years ago

in your view: if ($month->includes($day)) {

echo $day

}

yohang commented 8 years ago

Hi,

The end is excluded from the period for a simple reason : How do you do in real life ? Do events usualy finish at 23:59:59.999999 ?

If including the end date, how do you define it ? By substracting a second ? Or a millisecond ? Or maybe a microsecond ? What is the smallest possible time-unit ? So the excluded end date is the "easy" way to do it, as the tests are simple. If begin <= date < end, we're in the period.

garak commented 8 years ago

Well, we're talking about a month, so the time unit is... guess what? a day. Who cares about seconds?

yohang commented 8 years ago

@garak What should return $february->contains(new \DateTime('2016-02-29.999500')) for exemple ? Do we agree that it should return true ? It seems obvious to me, so it implies the en bound of the period is excluded.