Make DurationBuilder public

[nscala-time]

Make DurationBuilder public

[memelet]

Hmm, here's another problem with DurationBuilder (but maybe not related to the protection?)

    val dur = 10.minutes
    val exp = new Interval((iHour.start + 20.minutes), dur.toDuration)

Yields Error:(34, 9) Result type in structural refinement may not refer to a user-defined value class
val dur = 10.minutes
^

This is because of "structural types cannot use value classes in method parameter or return types" from http://docs.scala-lang.org/overviews/core/value-classes.html

I'm guessing this is because DurationBuilder now extends AnyVal?

[memelet]

Another example from our code

  implicit class DurationBuilderW(db1: DurationBuilder) {
    def to(db2: DurationBuilder): (Duration, Duration) = (db1, db2)
    def to(db2: Duration): (Duration, Duration) = (db1, db2)
    def or_greater: (Duration, Duration) = (db1, 1000.hours.toStandardDuration)
  }
  ...
  classify((0.seconds to 5.minutes,  as (Shutdown)),
           (5.minutes to 10.minutes, as (Shutdown)),
           (10.seconds or_greater,   as (Shutdown)))

(I am trying to upgrade from scala-time to nscala-time, but we were using a local fork with DurationBuilder public. I'm trying to not need a local fork...)

[gakuzzzz]

I think that DurationBuilder is Intermediate representation, so It should not be public.
We use Period or Duration usually.

However, It is inexpediency that 15.hours + 30.minutes can not assign to Duration or Period parameter.

Should we make DurationBuilder#toPeriod implicit?

[gakuzzzz]

Oh, These has been defined.
https://github.com/nscala-time/nscala-time/blob/releases/0.8.0/src/main/scala/com/github/nscala_time/time/Implicits.scala#L36-L39

haveTheSameDurationAs has two parameter type.
so compiler can not decide using witch of them.

def haveTheSameDurationAs(expected: Period) seems a bit danger.
Whould you use only haveTheSameDurationAs(expected: Duration)?

[memelet]

As for the haveTheSameDurationAs I just got rid of the overloading (a good idea in a any case). That forces the compiler to convert the DurationBuilder.

But we have tried nearly every permutation of implicit conversions in our fork of scala-time, and all gave problem of some kind or another.

We have other extension classes like:

  implicit class IntervalW(interval: Interval) {
    def |<< (duration: Duration) = interval.withStart(interval.start.minus(duration))
    def |>> (duration: Duration) = interval.withStart(interval.start.plus(duration))
    ..
  }
  implicit class DurationBuilderW(db: DurationBuilder) extends DurationOpsW {
    def |<< (interval: Interval) = interval.withStart(interval.start.minus(duration))
    def |>> (interval: Interval) = interval.withStart(interval.start.plus(duration))
    ..
  }

Which allows us to do things like:

    (1.hour |<< interval_2  >>| 1.hour) must_== (interval_1 ++ interval_3)
    (1.hour |>> interval_24 <<| 1.day) must_== ...

This is not possible (as far as I can tell) without being able to define DurationBuidlerW.

I understand if you don't want to make DurationBuilder public. If so we will just maintain our fork and work on pulling out just the bits of scala-time we use.

[memelet]

Here's something else that fails

    val classifyCurrentMaxDuration = 10.seconds
    val x = classifyCurrentMaxDuration - 1.second

Error:(122, 45) ambiguous reference to overloaded definition, 
both method - in class DurationBuilder of type (amount: org.joda.time.ReadableDuration)org.joda.time.Duration
and  method - in class DurationBuilder of type (period: org.joda.time.ReadablePeriod)org.joda.time.Period
match argument types (com.github.nscala_time.time.DurationBuilder)
        val x = (classifyCurrentMaxDuration - 1.second)
                                            ^

I have to say that DurationBuilder has been a pain for years...

[gakuzzzz]

Wow, It's a so embarrassing that 10.seconds - 1.second can not compile.

nscala-time/src/main/scala/com/github/nscala_time/time/DurationBuilder.scala

Lines 31 to 32 in b39114b

	def +(that: DurationBuilder): DurationBuilder =
	DurationBuilder(this.underlying.plus(that.underlying))

It seems absent def - (DurationBuilder)

@kmizu @xuwei-k
Do you konw the reason why def - (DurationBuilder) is absent in DurationBuilder ?

I agree that DurationBuilder pains us.
Still, I think that make DurationBuilder public is bad idea.
Instead, we might remove DurationBuilder and provide another way that build Duration.
What do you think?　> all

[xuwei-k]

It's a so embarrassing that 10.seconds - 1.second can not compile.

I'm going to send pull request soon.

[xuwei-k]

#53

[memelet]

If the methods on duration builder were on both Duration and Period that would be big improvement, for us at least. But then our system rarely does things like 10.days + 2.hours.

[gakuzzzz]

I have merged it.

[kmizu]

@memelet I'll try to implement your request as my PR (as long as I don't make DurationBuilder public). Wait a while.

[kmizu]

Fixed via #64