Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:68306
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.42 as permitted sender)
MIME-Version: 1.0
Sender: jakub.php@gmail.com
In-Reply-To: <CAJWj5+HcwqxdiZm4ZOdgGj1KB7fo_qFM1S6sfRJ_0LdBr5XRmQ@mail.gmail.com>
References: <CAJWj5+Es5qShZze6jLmFm2V-Tv5BkuFXx=zwbzRmGsor0yPYrw@mail.gmail.com>
	<alpine.DEB.2.10.1306281208280.28539@whisky.home.derickrethans.nl>
	<CAJWj5+GQNLb4pUEnc3J8J13pA5Lv_UDcKTz_A4F0GE7xwbnv3Q@mail.gmail.com>
	<alpine.DEB.2.10.1306281546070.5110@whisky.home.derickrethans.nl>
	<CAJWj5+ENhPG8cPARU3SBREU_yNFbwhmr+aT675+Q+LWKLng30A@mail.gmail.com>
	<CAEKnhAH_o2k+cBTkrSzC3_468aPQP_HnVeO41ssJOWu2ULeoVA@mail.gmail.com>
	<CAJWj5+HcwqxdiZm4ZOdgGj1KB7fo_qFM1S6sfRJ_0LdBr5XRmQ@mail.gmail.com>
Date: Thu, 25 Jul 2013 18:28:27 +0100
Message-ID: <CAEKnhAFF9hKV=VdjyYXu7c-=M4WjEUvRZYk_9E_jEa3RpE8ffw@mail.gmail.com>
To: Simon Schick <simonsimcity@gmail.com>
Cc: Derick Rethans <derick@php.net>, PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11c1bfea9ec5bf04e2595aa6
Subject: Re: [PHP-DEV] Adding a time interval to an interval
From: bukka@php.net (Jakub Zelenka)

--001a11c1bfea9ec5bf04e2595aa6
Content-Type: text/plain; charset=ISO-8859-1

On Mon, Jul 22, 2013 at 8:24 PM, Simon Schick <simonsimcity@gmail.com>wrote:

> On Sun, Jul 21, 2013 at 6:14 PM, Jakub Zelenka <bukka@php.net> wrote:
> > On Tue, Jul 2, 2013 at 8:21 AM, Simon Schick <simonsimcity@gmail.com>
> wrote:
> >>
> >> On Fri, Jun 28, 2013 at 11:20 PM, Derick Rethans <derick@php.net>
> wrote:
> >> >
> >> > Hey Simon,
> >> >
> >> > PS, please don't top-reply as per mailinglist guidelines:
> >> > http://us2.php.net/reST/README.MAILINGLIST_RULES
> >> >
> >> > On Fri, 28 Jun 2013, Simon Schick wrote:
> >> > > On Fri, Jun 28, 2013 at 7:10 PM, Derick Rethans <derick@php.net>
> >> > > wrote:
> >> > >
> >> > > > On Fri, 28 Jun 2013, Simon Schick wrote:
> >> > > >
> >> > > > > I'd like to extend the API of php by a method.
> >> > > >
> >> > > > Great, let me know if you need any help with the implementation,
> or
> >> > > > when you have any questions on how it currently works.
> >> > >
> >> > > Sorry - I that was easy to missunderstand ;) I thought of providing
> >> > > the idea. But I would happily also try to implement it by my own -
> >> > > just to also have contributed to the php-core. If you could give me
> a
> >> > > hint where to start - I feel quite lost in the code right now ...
> even
> >> > > so I read the great tutorials about php-internals provided by
> Anthony
> >> > > Ferrara.
> >> > >
> >> > > And this may take some time! I've just started with C/C++.
> >> > >
> >> > > Do you think this is an easy task to start? Depending on knowledge
> of
> >> > > php-internals ;)
> >> >
> >> > It's not that difficult, but not the easiest thing either. Basically,
> >> > you have to do the following steps:
> >> >
> >> > 1. in ext/date/lib/interval.c add a function that has two arguments of
> >> >    the timelib_rel_time* type. It should return a *new*
> >> >    timelib_rel_time* structure that has the two intervals added up
> >> >    together. It shouldn't change either of the original
> >> >    timelib_rel_time* arguments. Add the function definition to
> >> >    ext/date/lib/timelib.h too.
> >> > 2. You can only add two timelib_rel_time* arguments if neither of
> >> >    first_last_day_of, special, have_weekday_relative, and
> >> >    have_special_relative are set. You can'd add a "first day of next
> >> >    month" to a "+5 days" interval - it only works for simple y:m:d
> h:i:s
> >> >    intervals.
> >> > 3. In ext/date/php_date.c add:
> >> >
> >> >    a. a new ARG_INFO struct after the one for
> >> >       arginfo_date_interval_construct,
> >> >    b. Add a new method, "add" after
> >> >       PHP_ME_MAPPING(createFromDateString,
> >> > date_interval_create_from_date_string,...
> >> >    c. Add a new PHP_FUNCTION(date_interval_add) after the function
> >> >       PHP_FUNCTION(date_interval_format) that takes two DateInterval
> >> >       objects, extract the timelib_rel_time information, call the new
> >> >       function that you've added in step 1, and replace the
> >> >       timelib_rel_time* that is part of the DateInterval object with
> the
> >> >       returned value.
> >> >    d. Add a forwards declaration to php_date.h after
> >> >       PHP_FUNCTION(date_interval_create_from_date_string);
> >> >
> >> > 4. Make sure that the function can be called both as a procedural way
> >> >    (date_interval_add) and an object oriented way
> (DateInterval->add()).
> >> >
> >> > 5. Write test cases and put them in ext/date/tests.
> >> >
> >> > That's what I can think off right now.
> >> >
> >> > cheers,
> >> > Derick
> >> >
> >> >
> >> >
> >> > --
> >> > http://derickrethans.nl | http://xdebug.org
> >> > Like Xdebug? Consider a donation: http://xdebug.org/donate.php
> >> > twitter: @derickr and @xdebug
> >> > Posted with an email client that doesn't mangle email: alpine
> >>
> >> Hi, Derick
> >>
> >> I thought a bit about the changes here and came up with the following
> >> feedback:
> >>
> >> If I can't add intervals where either one is relative, I'd also add a
> >> method isRelative() to the class, where the user can check if it's a
> >> relative interval.
> >> But anyways - what happens if the interval is relative and you try to
> >> access one of the public properties? They're all designed for static
> >> intervals (let's say +5days), right?
> >>
> >> What, if the user tries to add a relative to a fixed interval? Should
> >> I trigger a E_WARNING, because it's no breaking error, but the value
> >> can't be calculated?
> >>
> >>
> >> Bye,
> >> Simon
> >>
> >> --
> >> PHP Internals - PHP Runtime Development Mailing List
> >> To unsubscribe, visit: http://www.php.net/unsub.php
> >>
> >
> >
> > Hi,
> >
> >
> > here is the patch: https://github.com/php/php-src/pull/390
> >
> > It's basically what Derick suggested. However there is one issue that
> needs
> > to be resolved - interval normalization (timelib_do_rel_normalize). The
> > problem is that currently there is no way how to get a base timelib_time
> > from the interval. It means that it's not possible to find out how many
> days
> > use for normalization. For example if interval1 is 20 days and interval2
> is
> > 21 days, then the resulted normalized interval will be 1 month and ??
> > days... :)
> >
> > There are few solution that come to my mind:
> >
> > 1. no normalization (currently implemented) - this is the easiest
> solution
> > but the result from the example will be 0 months and 41 days can give
> > interval that has over 31 days...
> > 2. warning if the resulted interval has more than 28 days (February :) )
> > 3. save base timelib_time with the interval when created and then use it
> as
> > a base class - it will probably require some sort of copying or ref
> counting
> > but it seems like the most logical solution
> >
> > What do you think?
> >
> > Jakub
>
> Hi, Jakub
>
> Many thanks for the implementation. I haven't had the time to do it,
> but would really have enjoyed it :)
>
> Now, I'll contribute by a comment:
> I think, the most valuable result would be the first option.
>
> Reason: I can also create a date-interval for "90 days".
>
> Here's my example:
> php > var_dump(DateInterval::createFromDateString("90 days"));
> class DateInterval#3 (8) {
>   public $y =>
>   int(0)
>   public $m =>
>   int(0)
>   public $d =>
>   int(90)
>   public $h =>
>   int(0)
>   public $i =>
>   int(0)
>   public $s =>
>   int(0)
>   public $invert =>
>   int(0)
>   public $days =>
>   int(0)
> }
>
> Adding or substracting some interval from this wouldn't be that
> different as something like this:
>
> php > var_dump(DateInterval::createFromDateString("90 days and -3
> months"));
> class DateInterval#3 (8) {
>   public $y =>
>   int(0)
>   public $m =>
>   int(-3)
>   public $d =>
>   int(90)
>   public $h =>
>   int(0)
>   public $i =>
>   int(0)
>   public $s =>
>   int(0)
>   public $invert =>
>   int(0)
>   public $days =>
>   int(0)
> }
>
> I think, we shouldn't care, as long as both intervals are not relative
> (like "next monday").
>
> Btw ... wouldn't be that hard to add a "sub()" method to the class,
> substracting a DateInterval ;)
>
> Bye
> Simon
>

Hi Simon,

Thanks for the feedback. I have just added the sub method:
https://github.com/bukka/php-src/commit/88824d475cf8e8dd9b4fa4a09cb252f2f441acad

We just need to wait for Derick when he gets time to review it :) Think
that this is for 5.6 so we have got lots of time anyway... :)

Cheers

Jakub

--001a11c1bfea9ec5bf04e2595aa6--