benplummer / calendarful

Recurring event occurrences generator and organiser.
MIT License
113 stars 9 forks source link

Generated occurrences are RecurrentEventInterface clones. #4

Open benplummer opened 9 years ago

benplummer commented 9 years ago

Currently in the default package Recurrence Types, when an occurrence of a Recurrent Event is generated it is a clone of the Recurrent Event as shown here: https://github.com/benplummer/calendarful/blob/master/src/Recurrence/Type/Daily.php#L93

This means that every occurrence will be an implementation of the RecurrentEventInterface. This is not ideal as only Recurrent Events should follow that interface.

Generated occurrences should be instances of the standard EventInterface as occurrences of Recurrent Events ideally should not recur themselves.

The default Recurrence Type code does call methods which should have implementations that remove recurrent related property values (https://github.com/benplummer/calendarful/blob/master/src/Recurrence/Type/Daily.php#L102-L108) however this makes assumptions that the user is going to do this. If they don't, occurrences could also be recurrent which may not have nice results.

Ideally, occurrences should be implementations of the EventInterface but the solution is not clear.

Custom Recurrence Types that users inject into the package are fine as they can address app code and instantiate models explicitly. The issue resides in the default package Recurrence Types that are based on Event abstractions.

NigelGreenway commented 9 years ago

Have you thought about having the event as a value object? You can then then have an EventGeneratorInterface which will always return an Event value object. Will put some code together when i get chance.

EDIT: Merged comments

NigelGreenway commented 9 years ago

Ignore the ValueObject idea, I value object has no identity so that will not work...