On Tue, Oct 20, 2009 at 7:01 AM, Derek Chen-Becker <dchenbec...@gmail.com>wrote:

> Here is my argument for breaking it out:
>
> I think that we're in agreement that the *default* impl for Lift should
> eventually be Joda Time, after some period of deprecation on the old stuff.


What I checked in allows you to use JodaTime just as easily (well with 2
extra characters in a few method names) as java.util.Date.  How is anything
more "default" than that?


> I also agree with your earlier comment that it should be something that the
> developer can choose. I'm sure there will be cases where people want to use
> java.util instead of Joda Time because of preference or constraint (say, no
> external jars). The problem is that "import Helpers._" is a very common
> import, and by having TimeHelpers be part of Helpers it makes it very
> difficult to do a clean separation of impls.


It's an interesting difference between an OO vs. non-OO.  In the
implementation I created, there choice of one or the other is made based on
singleton methods invoked.  This allows mixing both in the same code simply
by invoking now or jtNow.


> My main example is the time manipulation DSL. We can't mask the implicit
> defs that are already part of TimeHelpers, so it's very difficult (maybe
> impossible) to define a Joda Time based DSL (which would properly handle
> things like daylight savings time, etc).


I'm unclear why this is not possible.  We can add a DSL for manipulating
JodaTime without breaking anything we have.  The TimeSpan class simply gets
enhanced to deal with additional stuff and maybe uses JodaTime under the
covers.


> By removing TimeHelpers from Helpers, it means that the user can have
> absolute control over which time handling impl they're using without having
> to give up all of the nice things in Helpers.


They have that now with the implementation I did on your branch.




> It would be a breaking change to code, but it would be pretty minor, since
> the fix is just to add an import to the code that breaks. Here is my
> proposal for how we change things:
>
>
>    - *version 1.1-final*:
>
>
>    1. Rename the current TimeHelpers trait to JavaTimeHelpers and add a
>       new TimeHelpers trait that extends JavaTimeHelpers, providing an alias. 
> This
>       means no breaking code in this release.
>       2. Create a new JodaTimeHelpers that mirrors the non-deprecated
>       functionality of JavaTimeHelpers (there were already some deprecated 
> methods
>       on TimeHelpers that could go away).
>       3. We can also copy back some of the new methods on JodaTimeHelpers
>       and refactor them for java.util (I'm thinking epoch and some other minor
>       ones).
>       4. Add a JavaTimeFormatting and JodaTimeFormatting trait (currently
>       TimeRules in my code) that allows for custom formats and format 
> functions of
>       dates and times. The reason I want to split out the impls here, just 
> like
>       Java/JodaTimeHelpers, is so that Joda can be an optional dependency for
>       people who can't use it.
>
>
>    - *version 1.2-final*:
>
>
>    1. Remove the TimeHelpers trait and remove TimeHelpers from Helpers. At
>    this point, people would have to explicitly choose JavaTimeHelpers or
>    JodaTimeHelpers in their import statement to determine which impl they're
>    using.
>
> The main thing is that having TimeHelpers tied to Helpers makes it very
> difficult to give the user a choice of impls, something that I feel strongly
> goes against how Lift does things otherwise.
>
> Derek
>
>
> On Mon, Oct 19, 2009 at 10:36 PM, David Pollak <
> feeder.of.the.be...@gmail.com> wrote:
>
>> I have a strong preference not to break the TimeHelpers out of Helpers.
>> I'm not seeing the problem with additional methods on TimeHelpers that do
>> the JodaTime thing.  Can you guys help me understand your point of view.
>>
>>
>> On Mon, Oct 19, 2009 at 8:16 PM, Derek Chen-Becker <dchenbec...@gmail.com
>> > wrote:
>>
>>> Oh, that was a sidetrack. I was thinking that it could help enforce the
>>> common contract, but return types are different for the same method names so
>>> that really wouldn't work. I really just want to try and separate them out,
>>> or provide a different "Helpers" that provides JodaTime stuff under the same
>>> method names. My goal is really to minimize code changes, so one or two
>>> imports instead of a lot of find/replace (as simple as that may be) would be
>>> preferable in my book.
>>>
>>> Derek
>>>
>>>
>>> On Mon, Oct 19, 2009 at 8:53 PM, Naftoli Gugenheim <naftoli...@gmail.com
>>> > wrote:
>>>
>>>>
>>>> What would be the purpose of having a common trait though?
>>>>
>>>> -------------------------------------
>>>> Derek Chen-Becker<dchenbec...@gmail.com> wrote:
>>>>
>>>> That was pretty much what I was trying to communicate with my last
>>>> email,
>>>> just not very effectively.
>>>>
>>>> Derek
>>>>
>>>> On Mon, Oct 19, 2009 at 7:14 PM, Naftoli Gugenheim <
>>>> naftoli...@gmail.com>wrote:
>>>>
>>>> >
>>>> > Since there are anyway minor breaking changes in 1.1, maybe it's worth
>>>> it
>>>> > to take TimeHelpers out of Helpers. This way, it will simply require
>>>> an
>>>> > extra import, of either TimeHelpers or JodaHelpers, which can be
>>>> chosen by
>>>> > the individual developer.
>>>> > Whenever someone is ready to migrate, they will be able to do so on a
>>>> > file-by-file (or import-scope-by-import-scope) basis.
>>>> > Eventually JodaHelpers could be included in Helpers.
>>>> >
>>>> >
>>>> > -------------------------------------
>>>> > Derek Chen-Becker<dchenbec...@gmail.com> wrote:
>>>> >
>>>> > Along those same lines, maybe there should be a common trait that
>>>> doesn't
>>>> > define an impl, and then have separate Joda and java.util impl traits
>>>> that
>>>> > don't mix directly into Helpers.
>>>> >
>>>> > On Mon, Oct 19, 2009 at 6:57 PM, Derek Chen-Becker <
>>>> dchenbec...@gmail.com
>>>> > >wrote:
>>>> >
>>>> > > What I was thinking earlier is that we can simply make a JodaHelpers
>>>> > object
>>>> > > that mixes in JodaTimeHelpers instead of TimeHelpers. That way, code
>>>> > changes
>>>> > > to move to Joda Time would mostly just be an import change instead
>>>> of
>>>> > having
>>>> > > to alter every instance of now ⇒ jtNow.
>>>> > >
>>>> > > Derek
>>>> > >
>>>> > >
>>>> > > On Mon, Oct 19, 2009 at 6:12 PM, David Pollak <
>>>> > > feeder.of.the.be...@gmail.com> wrote:
>>>> > >
>>>> > >> We'll deprecate the Date/Calendar stuff.  I'm not sure we're going
>>>> to
>>>> > >> remove it any time soon (in the next year +).  Personally, I don't
>>>> see a
>>>> > >> compelling reason to remove it from the code base (there may be
>>>> people
>>>> > who
>>>> > >> would prefer not to use JodaTime).  So, we might need better names
>>>> than
>>>> > >> jtNow, etc., but I'm not expecting to make "now" return a DateTime.
>>>> > >>
>>>> > >>
>>>> > >> On Mon, Oct 19, 2009 at 5:03 PM, Derek Chen-Becker <
>>>> > dchenbec...@gmail.com
>>>> > >> > wrote:
>>>> > >>
>>>> > >>> Well, yes. I actually had similar code working, but what I was
>>>> trying
>>>> > to
>>>> > >>> avoid is a situation where someone has code like:
>>>> > >>>
>>>> > >>> import Helpers._
>>>> > >>> ..
>>>> > >>>
>>>> > >>> val someTime = now + ( 3 minutes)
>>>> > >>>
>>>> > >>> and then to use this code with JodaTime they need to change it to
>>>> > >>>
>>>> > >>> val someTime = jtNow + (3 minutes)
>>>> > >>>
>>>> > >>> and then when we remove the current java.util impl they would have
>>>> to
>>>> > >>> switch back to the original code. Also, because of all of these
>>>> issues
>>>> > I
>>>> > >>> haven't fully worked out things like JodaSpanBuilder, which should
>>>> > really be
>>>> > >>> using JodaTime's Periods instead of Durations since those take
>>>> into
>>>> > account
>>>> > >>> things like daylight savings time and leap years (none of the
>>>> current
>>>> > code
>>>> > >>> does that right now, java.util or JodaTime). If this is the best
>>>> > compromise
>>>> > >>> we can come up with I'll just make a new branch to finish working
>>>> on
>>>> > the
>>>> > >>> JodaTime side of things.
>>>> > >>>
>>>> > >>> Thanks,
>>>> > >>>
>>>> > >>> Derek
>>>> > >>>
>>>> > >>>
>>>> > >>> On Mon, Oct 19, 2009 at 4:57 PM, David Pollak <
>>>> > >>> feeder.of.the.be...@gmail.com> wrote:
>>>> > >>>
>>>> > >>>> Please take a look at
>>>> > >>>>
>>>> >
>>>> http://github.com/dpp/liftweb/commit/8b5ad6d7fac428886a74356150b332443e4443e7
>>>> > >>>>
>>>> > >>>> I think I've got JodaTime and the existing code playing nicely
>>>> > together.
>>>> > >>>>
>>>> > >>>>
>>>> > >>>> On Mon, Oct 19, 2009 at 2:23 PM, Derek Chen-Becker <
>>>> > >>>> dchenbec...@gmail.com> wrote:
>>>> > >>>>
>>>> > >>>>> wip-dcb-issue-89-jodatime. I just checked in everything I have,
>>>> which
>>>> > >>>>> means that branch currently doesn't compile. Maybe I'm
>>>> overthinking
>>>> > it, but
>>>> > >>>>> TimeHelpers does define some nice convenience methods and my
>>>> goal is
>>>> > to
>>>> > >>>>> replicate and extend that with Joda Time.
>>>> > >>>>>
>>>> > >>>>> Derek
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>> On Mon, Oct 19, 2009 at 3:13 PM, David Pollak <
>>>> > >>>>> feeder.of.the.be...@gmail.com> wrote:
>>>> > >>>>>
>>>> > >>>>>> What branch is the code on?
>>>> > >>>>>>
>>>> > >>>>>>
>>>> > >>>>>> On Mon, Oct 19, 2009 at 1:25 PM, Derek Chen-Becker <
>>>> > >>>>>> dchenbec...@gmail.com> wrote:
>>>> > >>>>>>
>>>> > >>>>>>> I spoke too soon. If I try to do masking imports, I get errors
>>>> > about
>>>> > >>>>>>> ambiguous defs:
>>>> > >>>>>>>
>>>> > >>>>>>> [WARNING]
>>>> > >>>>>>>
>>>> >
>>>> /home/software/liftweb/lift-util/src/test/scala/net/liftweb/util/JodaTimeHelpersSpec.scala:71:
>>>> > >>>>>>> error: reference to now is ambiguous;
>>>> > >>>>>>> [WARNING] it is imported twice in the same scope by
>>>> > >>>>>>> [WARNING] import _root_.net.liftweb.util.JodaTimeHelpers._
>>>> > >>>>>>> [WARNING] and import _root_.net.liftweb.util.TimeHelpers._
>>>> > >>>>>>> [WARNING]       3.seconds.later.millis must beCloseTo((now +
>>>> > >>>>>>> 3.seconds).millis, 100L)
>>>> > >>>>>>>
>>>> > >>>>>>> Since TimeHelpers is automatically part of Helpers, and
>>>> "import
>>>> > >>>>>>> Helpers._" is a pretty common import, it's going to require
>>>> some
>>>> > other
>>>> > >>>>>>> mechanism to get the proper import into scope. Any solution
>>>> which
>>>> > results in
>>>> > >>>>>>> "now" returning a java.util.Date essentially defeats the
>>>> purpose of
>>>> > moving
>>>> > >>>>>>> to Joda Time, since you can just do
>>>> > >>>>>>>
>>>> > >>>>>>>   /** transforms a java.util.Date to a org.joda.time.DateTime
>>>> */
>>>> > >>>>>>>   implicit def dateToDateTime(in : Date) : DateTime = new
>>>> > >>>>>>> DateTime(in)
>>>> > >>>>>>>
>>>> > >>>>>>>   /** transforms an org.joda.time.DateTime to a java.util.Date
>>>> */
>>>> > >>>>>>>   implicit def dateTimeToDate(in : DateTime) : Date =
>>>> in.toDate
>>>> > >>>>>>>
>>>> > >>>>>>> in your code and get most of it for free. What I'm trying to
>>>> > achieve
>>>> > >>>>>>> here is a refactoring of Lift's time formatting and processing
>>>> API
>>>> > that
>>>> > >>>>>>> lends itself to clean usage of JodaTime.
>>>> > >>>>>>>
>>>> > >>>>>>> Derek
>>>> > >>>>>>>
>>>> > >>>>>>>
>>>> > >>>>>>> On Mon, Oct 19, 2009 at 2:01 PM, Derek Chen-Becker <
>>>> > >>>>>>> dchenbec...@gmail.com> wrote:
>>>> > >>>>>>>
>>>> > >>>>>>>> The major issue is the time DSL. Everything else works fine
>>>> by
>>>> > >>>>>>>> masking the definitions with a later import, e.g.
>>>> > >>>>>>>>
>>>> > >>>>>>>> import Helpers._
>>>> > >>>>>>>> import JodaTimeHelpers._
>>>> > >>>>>>>>
>>>> > >>>>>>>> The problem is that implicit defs (views) don't mask, so I
>>>> can't
>>>> > >>>>>>>> override or mask it so that "3 seconds" returns a Joda Time
>>>> > Duration, which
>>>> > >>>>>>>> is a cleaner way of representing time durations in
>>>> milliseconds.
>>>> > In order to
>>>> > >>>>>>>> get a better discussion going, I'll go ahead and comment out
>>>> my
>>>> > refactored
>>>> > >>>>>>>> DSL and put up the code that does work this afternoon and we
>>>> can
>>>> > discuss it
>>>> > >>>>>>>> from there.
>>>> > >>>>>>>>
>>>> > >>>>>>>> Derek
>>>> > >>>>>>>>
>>>> > >>>>>>>>
>>>> > >>>>>>>> On Mon, Oct 19, 2009 at 1:37 PM, David Pollak <
>>>> > >>>>>>>> feeder.of.the.be...@gmail.com> wrote:
>>>> > >>>>>>>>
>>>> > >>>>>>>>> I've just looked at the code.
>>>> > >>>>>>>>>
>>>> > >>>>>>>>> There's nothing that's going to be incompatible moving to
>>>> > >>>>>>>>> JodaTime.  Sure, the month method will have a different
>>>> result if
>>>> > you pass
>>>> > >>>>>>>>> it a JodaTime instance than in you pass it a Date instance.
>>>>  If
>>>> > this is
>>>> > >>>>>>>>> documented, then it's not a problem.
>>>> > >>>>>>>>>
>>>> > >>>>>>>>> Existing methods don't get changed.  For methods that return
>>>> a
>>>> > Date
>>>> > >>>>>>>>> or Calendar, there'll be a parallel method (e.g., jNow or
>>>> > somesuch) that
>>>> > >>>>>>>>> returns a JodaTime instance.
>>>> > >>>>>>>>>
>>>> > >>>>>>>>> Existing methods that take a Date or Calendar, there'll be
>>>> > parallel
>>>> > >>>>>>>>> methods that take a JodaTime instance.
>>>> > >>>>>>>>>
>>>> > >>>>>>>>> Where does this seem unworkable?
>>>> > >>>>>>>>>
>>>> > >>>>>>>>>
>>>> > >>>>>>>>>
>>>> > >>>>>>>>>
>>>> > >>>>>>>>> On Mon, Oct 19, 2009 at 12:26 PM, Jeppe Nejsum Madsen <
>>>> > >>>>>>>>> je...@ingolfs.dk> wrote:
>>>> > >>>>>>>>>
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> Derek Chen-Becker <dchenbec...@gmail.com> writes:
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> > That's pretty much my take. The whole Java
>>>> > >>>>>>>>>> Calendar/Date/Timezone impl is
>>>> > >>>>>>>>>> > poorly designed, hence Joda Time.
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> >  Now I've run into another wall, this time with the
>>>> > >>>>>>>>>> TimeSpanBuilder. I can't
>>>> > >>>>>>>>>> > mask the implicits from long/int to TimeSpanBuilder, so I
>>>> > can't
>>>> > >>>>>>>>>> define the
>>>> > >>>>>>>>>> > same DSL for things like "3 seconds", etc. I tried to
>>>> provide
>>>> > an
>>>> > >>>>>>>>>> implicit
>>>> > >>>>>>>>>> > conversion from TimeSpan to JodaSpan but that won't cover
>>>> all
>>>> > of
>>>> > >>>>>>>>>> the cases
>>>> > >>>>>>>>>> > and this is feeling increasingly ugly as I add more and
>>>> more
>>>> > >>>>>>>>>> layers and
>>>> > >>>>>>>>>> > implicit defs to work around the old API.
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> I haven't looked very closely at the current code, so don't
>>>> > really
>>>> > >>>>>>>>>> know
>>>> > >>>>>>>>>> what the issues are..... but this won't stop me from
>>>> commenting.
>>>> > >>>>>>>>>> I'm
>>>> > >>>>>>>>>> really looking forward to get rid of Calendar and
>>>> friends....
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> > At this point, the only way that this may work is if I
>>>> create
>>>> > >>>>>>>>>> and entirely
>>>> > >>>>>>>>>> > new JodaHelpers object that extends all of the Helpers
>>>> traits,
>>>> > >>>>>>>>>> replacing
>>>> > >>>>>>>>>> > TimeHelpers with my new JodaTimeHelpers trait. This may
>>>> be the
>>>> > >>>>>>>>>> path of least
>>>> > >>>>>>>>>> > pain, but it means two cycles of deprecation: one to
>>>> deprecate
>>>> > >>>>>>>>>> the current
>>>> > >>>>>>>>>> > TimeHelpers in favor of JodaTimeHelpers, and the second
>>>> to
>>>> > >>>>>>>>>> deprecate
>>>> > >>>>>>>>>> > JodaTimeHelpers back to a refactored/reworked
>>>> TimeHelpers.
>>>> > Does
>>>> > >>>>>>>>>> this sound
>>>> > >>>>>>>>>> > like a reasonable approach, or is this getting too crazy?
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> Depending on the cycle-length, this doesn't sound too bad.
>>>> But
>>>> > if
>>>> > >>>>>>>>>> it is
>>>> > >>>>>>>>>> 1.3 before we get pure JodaTime, this sounds excessive.
>>>> Lift 1.1
>>>> > >>>>>>>>>> already
>>>> > >>>>>>>>>> has numerous breaking changes so if a cycle is just a
>>>> milestone
>>>> > >>>>>>>>>> release
>>>> > >>>>>>>>>> then I think it's fine.
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> >  The other option I thought of is to make a new branch
>>>> just
>>>> > for
>>>> > >>>>>>>>>> this and
>>>> > >>>>>>>>>> > just track master with the branch. The big downside is
>>>> that
>>>> > this
>>>> > >>>>>>>>>> doubles the
>>>> > >>>>>>>>>> > release work to have a Joda Lift and non-Joda Lift.
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> Doesn't sound like a viable approach. We'll be carrying
>>>> this
>>>> > >>>>>>>>>> baggage
>>>> > >>>>>>>>>> forever (unless this was only a temp solution?)
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>> /Jeppe
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>>
>>>> > >>>>>>>>>
>>>> > >>>>>>>>>
>>>> > >>>>>>>>> --
>>>> > >>>>>>>>> Lift, the simply functional web framework
>>>> http://liftweb.net
>>>> > >>>>>>>>> Beginning Scala http://www.apress.com/book/view/1430219890
>>>> > >>>>>>>>> Follow me: http://twitter.com/dpp
>>>> > >>>>>>>>> Surf the harmonics
>>>> > >>>>>>>>>
>>>> > >>>>>>>>>
>>>> > >>>>>>>>>
>>>> > >>>>>>>
>>>> > >>>>>>>
>>>> > >>>>>>>
>>>> > >>>>>>>
>>>> > >>>>>>
>>>> > >>>>>>
>>>> > >>>>>> --
>>>> > >>>>>> Lift, the simply functional web framework http://liftweb.net
>>>> > >>>>>> Beginning Scala http://www.apress.com/book/view/1430219890
>>>> > >>>>>> Follow me: http://twitter.com/dpp
>>>> > >>>>>> Surf the harmonics
>>>> > >>>>>>
>>>> > >>>>>>
>>>> > >>>>>>
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>>
>>>> > >>>>
>>>> > >>>>
>>>> > >>>> --
>>>> > >>>> Lift, the simply functional web framework http://liftweb.net
>>>> > >>>> Beginning Scala http://www.apress.com/book/view/1430219890
>>>> > >>>> Follow me: http://twitter.com/dpp
>>>> > >>>> Surf the harmonics
>>>> > >>>>
>>>> > >>>>
>>>> > >>>>
>>>> > >>>
>>>> > >>>
>>>> > >>>
>>>> > >>
>>>> > >>
>>>> > >> --
>>>> > >> Lift, the simply functional web framework http://liftweb.net
>>>> > >> Beginning Scala http://www.apress.com/book/view/1430219890
>>>> > >> Follow me: http://twitter.com/dpp
>>>> > >> Surf the harmonics
>>>> > >>
>>>> > >> >>
>>>> > >>
>>>> > >
>>>> >
>>>> >
>>>> >
>>>> > >
>>>> >
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>> --
>> Lift, the simply functional web framework http://liftweb.net
>> Beginning Scala http://www.apress.com/book/view/1430219890
>> Follow me: http://twitter.com/dpp
>> Surf the harmonics
>>
>>
>>
>
> >
>


-- 
Lift, the simply functional web framework http://liftweb.net
Beginning Scala http://www.apress.com/book/view/1430219890
Follow me: http://twitter.com/dpp
Surf the harmonics

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Lift" group.
To post to this group, send email to liftweb@googlegroups.com
To unsubscribe from this group, send email to 
liftweb+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/liftweb?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to