I was finishing off phasing in new content and features of my latest project, the Surrey International Writers’ Conference (www.siwc.ca) website when some reports came in of problems with the published dates of the various writing workshops being held.
Workshops all adhere to a common schedule for each day of the 4-day conference, so I created a custom TimeSlot WordPress post type to centralize timeslots to allow sorting and grouping of workshops together on the master schedule. The data entry page for each workshop has a drop-down list box with friendly names like “Friday 10:00 AM – 11:30 PM”.
The actual format of the workshop date identifier string is “dd-hhmm-hhmm”, where dd is the day (05 = Friday, 06=Saturday, 07=Sunday, etc.) and the hhmm values specified the start and end times for the workshop. Our example workshop TimeSlot ID string as stored in the workshop metadata in WordPress would be “05-1000-1130”.
I had arbitrarily assigned Day 5 to be the Friday of the conference. This allowed the conference in future to potentially start earlier, say on Wednesday (3), or Thursday (4) to accommodate master classes or other pre-conference activities without needing to re-enter new timeslots every year (timeslots do not change year by year).
So when calculating the actual date of the workshop to display to users, I needed to subtract 5 days from the global start date of the conference (expressed as the date of the Friday), then add back the number of days in the workshop date structure. For example, for a Saturday 10:00 AM – 11:30 AM workshop, the ID would be (06-1000-1130). Taking 5 days away from date of the Friday (October 20, 2017), then adding 6 days gives us October 21, 2017 as the date of the Saturday.
That’s where the problem arose…
Here’s the original code, taking 5 off the conference start date, $siwc_conference_start (DateTime object), then adding the workshop day value that I extracted previously from the string.
// $day = workshop day, 5=Friday, calculated previously // following line modifies $siwc_conference_start!!! $thisdate = $siwc_conference_start->sub(new DateInterval('P5D')); $thisdate = $thisdate->add(new DateInterval('P' . $day . 'D')); return $thisdate;
This caused no end of problems! I was getting seemingly random dates for the Friday – like October 23, October 21, etc.
After much tracing, I isolated it to the fact that the all-important $siwc_conference_start variable, which should have been, and needed to be, constant, was actually changing in value between calls! The only possibility was the sub() method call (and the add(), of course, upon later reflection) affecting the object directly rather than simply returning the result of the calculation.
This outlined an issue I hadn’t been aware of in PHP regarding the mutability of the DateTime class, which $siwc_conference_start was declared as.
The sub() method was actually acting upon the $siwc_conference_start object, subtracting 5 days from the conference, rather than returning an object with the 5 days subtracted from it and leaving the original alone.
The subsequent add() method was adding a different value back to the conference start date. So the conference start date value was bouncing between different values, some actually correct, depending on prior calls. This also explained why I hadn’t caught the bug in my earlier tests – my test data didn’t exercise the function sufficiently (either it had all Friday data, or I didn’t notice the discrepancy).
The fixed code:
// $day = workshop day, 5=Friday, calculated previously $thisdate = clone $siwc_conference_start; $thisdate->sub(new DateInterval('P5D')); $thisdate->add(new DateInterval('P' . $day . 'D')); return $thisdate;
This is one fix – using the clone keyword to create a new object containing a shallow copy of the $siwc_conference_start object’s contents. The sub() and add() methods will act on that copy, not affecting the original.
There is also a second fix: declaring $siwc_conference_start as a DateTimeImmutable class introduced in PHP 5.5. This new class is an admission that the original DateTime class implementation was flawed in allowing the original object to be changed. The new implementation in DateTimeImmutable returns a copy of the object as would be expected.
Overall, this highlights a couple of interesting things around object mutability, when it is expected, and when it is not. The original implementation breaks a couple of different programming language expectations…
- Caused by my having used other languages’ DateTime types. For example, the .NET DateTime is a value type, therefore would behave as expected in the first implementation.
- Caused by expectations that DateTime should be implemented best as a value object rather than reference object. There are few situations where DateTimes are likely to be better off as reference objects (e.g. objects meant to be shared) than value objects. It’s not that the implementation was technically wrong, but the expected usage by programmers did not align well with the initial design decisions for this class.
This is also a lesson for creating a proper test harness for functions like this that are easy to test. I could easily have created a sample set of timeslot IDs and corresponding results, and this would have been identified before going into production.