Regarding SVN commit r892904

[Bob Morley]

I had accidentally replied via nabble to the Ofbiz - Commits sub-form; so I am going to move my response to Ofbiz - Dev.  If anyone saw that, I am going to rework my response here because after some consideration and discussion in our office, I think we have a better proposal.

--

Part of this revision caused the TimeEntry to no longer be created for work performed on the ProductionRun task.  Was the comment out intentional?

Let's say this was an accident ...

I think it is valuable to be creating the TimeEntry for the WorkEffort.  It provides valuable information about a resource allocating time to the particular task.  The trouble with the original implementation is that the data model resource book seemed to have a single "time" for this purpose -- "hours" on TimeEntry along with "hours" on the WorkEffort (in terms of the total hours spent).

There are good reasons to be able to have separation on the Work Effort for various classifications of "time worked" on a specific Task.  Currently, there are two classifications "setup" and "task" hours (specified in milliseconds).  The trouble is that a single TimeEntry is not going to provide us with this split.

My initial recommendation was to create a new entity WorkEffortTimeEntry that would allow a single WorkEffort to be bound to many TimeEntry entities.  These entities could have different types so you would have the details of the materialized amount that is stored on the WorkEffort itself.  However, I think it would actually be better to consider going with two (or more) WorkEfforts and each of them having its own TimeEntry.

The idea here is if you have a production run and you have a Task which has some Setup and some Task work, then it really should be modeled as two dependent work efforts.  Each of those work efforts would have "estimated/actual" "hours" column as per the Data Model Resource Book volume 1.  We could use WorkEffort association to bind them together if you wanted to navigate a particular task that is related to others (naturally it could also be modeled as a separate ProductionRun if you were so inclined).

At any rate, by doing this it would be possible to have a setup, task, teardown and any other arbitrary set of "pieces" of a particular Work Effort.  Each of these would have their own estimate / actual (rather than being bound to two specific ones as it is now).  Moreover, the TimeEntry would be the details that produce the materialized "hours" on the work effort.

Would should also entertain changing both the "hours" on TimeEntry and the "estimatedMilliSeconds" / "actualMilliSeconds" to a generic "time" field with an associated Unit of Measure.  This would allow folks to do these things at the milli second granularity if they really want, otherwise any other "time" UOM could be used (I suspect hours would be the norm).

[Adrian Crum]

Set up and tear down would each be a separate work effort in my opinion.

We have the TimeDuration data type for the estimated/actual time.

-Adrian

Bob Morley wrote:
> I had accidentally replied via nabble to the Ofbiz - Commits sub-form; so I
> am going to move my response to Ofbiz - Dev.  If anyone saw that, I am going
> to rework my response here because after some consideration and discussion
> in our office, I think we have a better proposal.
>

[Bob Morley]

Adrian Crum wrote

We have the TimeDuration data type for the estimated/actual time.

Are you referring to org.ofbiz.base.util.TimeDuration?  The intent would seem to be that we would define fields in the entity model as (what results to a Long) and use this class for simple (manual) serialization / deserialization.  That would work well for WorkEffort fields as you have indicated.

However, in taking a quick glance at TimeDuration a couple of things struck me ...

1) I am forced to use a constructor that requires int for the components of the duration.  If I was to build a screen that allowed the user to indicate 4.5 hours (for example) I could not use this class to easily convert to my long value for persistence (if that is part of its intent).

2) The makeNegative usage looks incorrect in the "component" constructor -- say I am going to new TimeDuration(1,-2,0,0,0,0,0) I think my expectation is my duration would be 10 months (1 year - 2 months); but what would happen is since one of the components is negative it would negate all components and I believe I would have a duration of -14 months.  (In fairness, the only existing use case that would appear to use negative duration is in WorkEffortServices and it is using the constructor that takes a long).

Bottom-line; I agree I think we should leave these on WorkEffort as stored in milliseconds and leverage TimeDuration.  If either of these two points make sense; perhaps we should cut a minor JIRA to resolve them if you feel they warrant potential repair.

[Adrian Crum-2]

If you spend a little time with Work Effort's Estimated Milliseconds field, you will find that there is a lookup screen that will allow you to enter 4.5 hours.

TimeDuration will encode/decode long values. You don't need to use the int constructors.

If you were entering 10 months into a field, I'm sure you would enter it as 10 months, not one year plus negative 2 months. When someone asks me for the time, I usually don't reply "11:00 PM minus 35 minutes."

-Adrian


--- On Tue, 4/20/10, Bob Morley <[hidden email]> wrote:

> From: Bob Morley <[hidden email]>
> Subject: Re: Regarding SVN commit r892904
> To: [hidden email]
> Date: Tuesday, April 20, 2010, 9:49 PM
>
>
> Adrian Crum wrote:
> >
> > We have the TimeDuration data type for the
> estimated/actual time.
> >
>
> Are you referring to
> org.ofbiz.base.util.TimeDuration?  The intent would
> seem to be that we would define fields in the entity model
> as (what results
> to a Long) and use this class for simple (manual)
> serialization /
> deserialization.  That would work well for WorkEffort
> fields as you have
> indicated.
>
> However, in taking a quick glance at TimeDuration a couple
> of things struck
> me ...
>
> 1) I am forced to use a constructor that requires int for
> the components of
> the duration.  If I was to build a screen that allowed
> the user to indicate
> 4.5 hours (for example) I could not use this class to
> easily convert to my
> long value for persistence (if that is part of its
> intent).
>
> 2) The makeNegative usage looks incorrect in the
> "component" constructor --
> say I am going to new TimeDuration(1,-2,0,0,0,0,0) I think
> my expectation is
> my duration would be 10 months (1 year - 2 months); but
> what would happen is
> since one of the components is negative it would negate all
> components and I
> believe I would have a duration of -14 months.  (In
> fairness, the only
> existing use case that would appear to use negative
> duration is in
> WorkEffortServices and it is using the constructor that
> takes a long).
>
> Bottom-line; I agree I think we should leave these on
> WorkEffort as stored
> in milliseconds and leverage TimeDuration.  If either
> of these two points
> make sense; perhaps we should cut a minor JIRA to resolve
> them if you feel
> they warrant potential repair.
> --
> View this message in context: http://n4.nabble.com/Regarding-SVN-commit-r892904-tp2018084p2018379.html
> Sent from the OFBiz - Dev mailing list archive at
> Nabble.com.
>

[Bob Morley]

Lol of course not, I was just suggesting that strictly speaking that  
constructor misuses the makeNegative.  With classes like this it is  
probably better to have a single instance variable store the ms for  
the duration.  Just impressions when previewing; if there us no value  
let's ignore.

I may reply quarter to ten (10,-15).

I will take a look at the lookup screen as per your direction.  But I  
can't imagine it makes use of TimeDuration directly, it would have to  
convert 4.5 to long or to (4,30).

Bob

On 2010-04-21, at 1:26 AM, Adrian Crum <[hidden email]> wrote:

> If you spend a little time with Work Effort's Estimated Milliseconds  
> field, you will find that there is a lookup screen that will allow  
> you to enter 4.5 hours.
>
> TimeDuration will encode/decode long values. You don't need to use  
> the int constructors.
>
> If you were entering 10 months into a field, I'm sure you would  
> enter it as 10 months, not one year plus negative 2 months. When  
> someone asks me for the time, I usually don't reply "11:00 PM minus  
> 35 minutes."
>
> -Adrian
>
>
> --- On Tue, 4/20/10, Bob Morley <[hidden email]> wrote:
>
>> From: Bob Morley <[hidden email]>
>> Subject: Re: Regarding SVN commit r892904
>> To: [hidden email]
>> Date: Tuesday, April 20, 2010, 9:49 PM
>>
>>
>> Adrian Crum wrote:
>>>
>>> We have the TimeDuration data type for the
>> estimated/actual time.
>>>
>>
>> Are you referring to
>> org.ofbiz.base.util.TimeDuration?  The intent would
>> seem to be that we would define fields in the entity model
>> as (what results
>> to a Long) and use this class for simple (manual)
>> serialization /
>> deserialization.  That would work well for WorkEffort
>> fields as you have
>> indicated.
>>
>> However, in taking a quick glance at TimeDuration a couple
>> of things struck
>> me ...
>>
>> 1) I am forced to use a constructor that requires int for
>> the components of
>> the duration.  If I was to build a screen that allowed
>> the user to indicate
>> 4.5 hours (for example) I could not use this class to
>> easily convert to my
>> long value for persistence (if that is part of its
>> intent).
>>
>> 2) The makeNegative usage looks incorrect in the
>> "component" constructor --
>> say I am going to new TimeDuration(1,-2,0,0,0,0,0) I think
>> my expectation is
>> my duration would be 10 months (1 year - 2 months); but
>> what would happen is
>> since one of the components is negative it would negate all
>> components and I
>> believe I would have a duration of -14 months.  (In
>> fairness, the only
>> existing use case that would appear to use negative
>> duration is in
>> WorkEffortServices and it is using the constructor that
>> takes a long).
>>
>> Bottom-line; I agree I think we should leave these on
>> WorkEffort as stored
>> in milliseconds and leverage TimeDuration.  If either
>> of these two points
>> make sense; perhaps we should cut a minor JIRA to resolve
>> them if you feel
>> they warrant potential repair.
>> --
>> View this message in context: http://n4.nabble.com/Regarding-SVN-commit-r892904-tp2018084p2018379.html
>> Sent from the OFBiz - Dev mailing list archive at
>> Nabble.com.
>>
>
>
>

[Adrian Crum-2]

If you look through the dev mailing list for February you will see where Adam and I discussed the class's design. That should give you some insight into why things look the way they do.

Most of the work that went into TimeDuration was the result of this Jira issue:

https://issues.apache.org/jira/browse/OFBIZ-3125

Originally, the class supported a negative duration (Today @ 11:00PM to Today @ 10:25PM equals negative 35 mins) but Adam thought there was a need to support a negative value for a single field.

When I first researched the issue, I had suggested splitting duration values in the database (separate fields for hours, mins secs, etc) but it was recommended that we keep durations stored in the database as a long so that duration values can be compared in SQL queries. TimeDuration encodes/decodes long values that can be compared in SQL queries.

To summarize, TimeDuration is a class designed by committee.

If you have a better design, I would be interested in seeing it.

In the meantime, I'm working on a date/time library that includes an ElapsedTime class (an improved TimeDuration class) that follows Martin Fowler's Quantity pattern.

-Adrian

--- On Tue, 4/20/10, Robert Morley <[hidden email]> wrote:

> From: Robert Morley <[hidden email]>
> Subject: Re: Regarding SVN commit r892904
> To: "[hidden email]" <[hidden email]>
> Date: Tuesday, April 20, 2010, 10:48 PM
> Lol of course not, I was just
> suggesting that strictly speaking that constructor misuses
> the makeNegative.  With classes like this it is
> probably better to have a single instance variable store the
> ms for the duration.  Just impressions when previewing;
> if there us no value let's ignore.
>
> I may reply quarter to ten (10,-15).
>
> I will take a look at the lookup screen as per your
> direction.  But I can't imagine it makes use of
> TimeDuration directly, it would have to convert 4.5 to long
> or to (4,30).
>
> Bob
>
> On 2010-04-21, at 1:26 AM, Adrian Crum <[hidden email]>
> wrote:
>
> > If you spend a little time with Work Effort's
> Estimated Milliseconds field, you will find that there is a
> lookup screen that will allow you to enter 4.5 hours.
> >
> > TimeDuration will encode/decode long values. You don't
> need to use the int constructors.
> >
> > If you were entering 10 months into a field, I'm sure
> you would enter it as 10 months, not one year plus negative
> 2 months. When someone asks me for the time, I usually don't
> reply "11:00 PM minus 35 minutes."
> >
> > -Adrian
> >
> >
> > --- On Tue, 4/20/10, Bob Morley <[hidden email]>
> wrote:
> >
> >> From: Bob Morley <[hidden email]>
> >> Subject: Re: Regarding SVN commit r892904
> >> To: [hidden email]
> >> Date: Tuesday, April 20, 2010, 9:49 PM
> >>
> >>
> >> Adrian Crum wrote:
> >>>
> >>> We have the TimeDuration data type for the
> >> estimated/actual time.
> >>>
> >>
> >> Are you referring to
> >> org.ofbiz.base.util.TimeDuration?  The intent
> would
> >> seem to be that we would define fields in the
> entity model
> >> as (what results
> >> to a Long) and use this class for simple (manual)
> >> serialization /
> >> deserialization.  That would work well for
> WorkEffort
> >> fields as you have
> >> indicated.
> >>
> >> However, in taking a quick glance at TimeDuration
> a couple
> >> of things struck
> >> me ...
> >>
> >> 1) I am forced to use a constructor that requires
> int for
> >> the components of
> >> the duration.  If I was to build a screen
> that allowed
> >> the user to indicate
> >> 4.5 hours (for example) I could not use this class
> to
> >> easily convert to my
> >> long value for persistence (if that is part of
> its
> >> intent).
> >>
> >> 2) The makeNegative usage looks incorrect in the
> >> "component" constructor --
> >> say I am going to new TimeDuration(1,-2,0,0,0,0,0)
> I think
> >> my expectation is
> >> my duration would be 10 months (1 year - 2
> months); but
> >> what would happen is
> >> since one of the components is negative it would
> negate all
> >> components and I
> >> believe I would have a duration of -14
> months.  (In
> >> fairness, the only
> >> existing use case that would appear to use
> negative
> >> duration is in
> >> WorkEffortServices and it is using the constructor
> that
> >> takes a long).
> >>
> >> Bottom-line; I agree I think we should leave these
> on
> >> WorkEffort as stored
> >> in milliseconds and leverage TimeDuration. 
> If either
> >> of these two points
> >> make sense; perhaps we should cut a minor JIRA to
> resolve
> >> them if you feel
> >> they warrant potential repair.
> >> --View this message in context: http://n4.nabble.com/Regarding-SVN-commit-r892904-tp2018084p2018379.html
> >> Sent from the OFBiz - Dev mailing list archive at
> >> Nabble.com.
> >>
> >
> >
> >
>

[Jacopo Cappellato-4]

On Apr 20, 2010, at 11:25 PM, Bob Morley wrote:

>
> I had accidentally replied via nabble to the Ofbiz - Commits sub-form; so I
> am going to move my response to Ofbiz - Dev.  If anyone saw that, I am going
> to rework my response here because after some consideration and discussion
> in our office, I think we have a better proposal.
>
> --
>
> Part of this revision caused the TimeEntry to no longer be created for work
> performed on the ProductionRun task.  Was the comment out intentional?
>
> Let's say this was an accident ...

If I am not wrong at that time I disabled it because it was only partially implemented feature; it had to be enhanced and completed before it is usable.
However I agree that time entries are an important concept and would be a very nice feature to have, especially when we will have decent ui to enter them and possibly a specialpurpose manufacturing application where:
1) a user logs in
2) all the tasks assigned to the users, or to the fixed asset (workcenter) where the user works, appears
3) the user can enter the time spent on the task (time entries), the materials used and produced; any scrap etc...

Jacopo

>
> I think it is valuable to be creating the TimeEntry for the WorkEffort.  It
> provides valuable information about a resource allocating time to the
> particular task.  The trouble with the original implementation is that the
> data model resource book seemed to have a single "time" for this purpose --
> "hours" on TimeEntry along with "hours" on the WorkEffort (in terms of the
> total hours spent).
>
> There are good reasons to be able to have separation on the Work Effort for
> various classifications of "time worked" on a specific Task.  Currently,
> there are two classifications "setup" and "task" hours (specified in
> milliseconds).  The trouble is that a single TimeEntry is not going to
> provide us with this split.
>
> My initial recommendation was to create a new entity WorkEffortTimeEntry
> that would allow a single WorkEffort to be bound to many TimeEntry entities.
> These entities could have different types so you would have the details of
> the materialized amount that is stored on the WorkEffort itself.  However, I
> think it would actually be better to consider going with two (or more)
> WorkEfforts and each of them having its own TimeEntry.
>
> The idea here is if you have a production run and you have a Task which has
> some Setup and some Task work, then it really should be modeled as two
> dependent work efforts.  Each of those work efforts would have
> "estimated/actual" "hours" column as per the Data Model Resource Book volume
> 1.  We could use WorkEffort association to bind them together if you wanted
> to navigate a particular task that is related to others (naturally it could
> also be modeled as a separate ProductionRun if you were so inclined).
>
> At any rate, by doing this it would be possible to have a setup, task,
> teardown and any other arbitrary set of "pieces" of a particular Work
> Effort.  Each of these would have their own estimate / actual (rather than
> being bound to two specific ones as it is now).  Moreover, the TimeEntry
> would be the details that produce the materialized "hours" on the work
> effort.
>
> Would should also entertain changing both the "hours" on TimeEntry and the
> "estimatedMilliSeconds" / "actualMilliSeconds" to a generic "time" field
> with an associated Unit of Measure.  This would allow folks to do these
> things at the milli second granularity if they really want, otherwise any
> other "time" UOM could be used (I suspect hours would be the norm).
> --
> View this message in context: http://n4.nabble.com/Regarding-SVN-commit-r892904-tp2018084p2018084.html
> Sent from the OFBiz - Dev mailing list archive at Nabble.com.

[Bob Morley]

Adrian Crum-2 wrote

In the meantime, I'm working on a date/time library that includes an ElapsedTime class (an improved TimeDuration class) that follows Martin Fowler's Quantity pattern.

I am certainly not interested in re-inventing the wheel here.  I know in our solution we are going to allow entry against a ProductionRun task in hours and I will want to leverage this TimeDuration as you have in WorkEffortServices.

Way back in 2006 - 2007 there was talk of including Joda time into Ofbiz (at the time the thread dealt with wrapping up time related functionality in minilang).  Do you know why we decided to back-off Joda time (if there was a conscious decision there)?  My understanding is that JSR 310 (new java date/time apis) that has been proposed for Java 7 is largely based on Joda time.  Being that the lic. is favorable, I would have thought pulling in and using their Duration might make sense vs. maintaining our own TimeDuration and/or a new date/time library?  Very interested in your thoughts because this really appears to be something in your wheel house.

[Adrian Crum]

Joda Time claims to fix the problems with the Java date/time API, while
it duplicates the problems in the Java date/time API. The only real
advantage to Joda Time is the additional calendars - which OFBiz already
has in the ICU4J library.

The author of JSR-310 complains about Joda in the spec and claims that
JSR-310 is better than the Joda Time API. I don't believe JSR-310 will
become a part of the Java spec any time soon. JSR-275 is more mature and
has seen a lot more activity than JSR-310, yet even that doesn't have
enough votes to get it into the Java spec.

If we actually had a choice, I would vote for using JSR-310 over Joda
Time. The problem is, there is no library that I can find. So far it
appears to be a draft specification only.

In the meantime we could really use a simple set of date/time classes
that take the guesswork out of handling dates/times. I started with the
Quantity pattern and included some good ideas from JSR-275 and JSR-310.
I believe the result is easy to use and reliable.

Mini-language has a set-calendar element - maybe that is the end result
of the conversation you were thinking of.

-Adrian

Bob Morley wrote:

>
> Adrian Crum-2 wrote:
>> In the meantime, I'm working on a date/time library that includes an
>> ElapsedTime class (an improved TimeDuration class) that follows Martin
>> Fowler's Quantity pattern.
>>
>
> I am certainly not interested in re-inventing the wheel here.  I know in our
> solution we are going to allow entry against a ProductionRun task in hours
> and I will want to leverage this TimeDuration as you have in
> WorkEffortServices.
>
> Way back in 2006 - 2007 there was talk of including Joda time into Ofbiz (at
> the time the thread dealt with wrapping up time related functionality in
> minilang).  Do you know why we decided to back-off Joda time (if there was a
> conscious decision there)?  My understanding is that JSR 310 (new java
> date/time apis) that has been proposed for Java 7 is largely based on Joda
> time.  Being that the lic. is favorable, I would have thought pulling in and
> using their Duration might make sense vs. maintaining our own TimeDuration
> and/or a new date/time library?  Very interested in your thoughts because
> this really appears to be something in your wheel house.

[Bob Morley]

Adrian Crum wrote

Joda Time claims to fix the problems with the Java date/time API, while
it duplicates the problems in the Java date/time API. The only real
advantage to Joda Time is the additional calendars - which OFBiz already
has in the ICU4J library.

The author of JSR-310 complains about Joda in the spec and claims that
JSR-310 is better than the Joda Time API. I don't believe JSR-310 will
become a part of the Java spec any time soon. JSR-275 is more mature and
has seen a lot more activity than JSR-310, yet even that doesn't have
enough votes to get it into the Java spec.

If we actually had a choice, I would vote for using JSR-310 over Joda
Time. The problem is, there is no library that I can find. So far it
appears to be a draft specification only.

In the meantime we could really use a simple set of date/time classes
that take the guesswork out of handling dates/times. I started with the
Quantity pattern and included some good ideas from JSR-275 and JSR-310.
I believe the result is easy to use and reliable.

Mini-language has a set-calendar element - maybe that is the end result
of the conversation you were thinking of.

Ahh that is good info -- what made me think of it was a Java Posse podcast I had been listening to when they were going through some of the proposed Java 7 enhancements.  I had remembered them talking about Joda rather favorably at the time.  At any rate, I am more than happy to use TimeDuration in its current form and cut over to whatever you provide going forward.  If I made an changes I will post them up so we can review and discuss if they have any merit.

This conversation has forked quite a bit from its starting point.  I think we had agreed that it is probably better to handle "setup" as a separate WorkEffort rather than coupling setup and actual task time in a single WorkEffort.  Time constraints as they are, what I am going to do for our project is re-activate the creation of the TimeEntry and only make use of "task time".  If one of our customers is interested in "setup" they would create a related WorkEffort and manage it consistently with other WorkEffort entities (in regards to time tracking, estimate, and task time).

Once I have something complete I will wrap it up in a patch for you guys to review.  This would include UI elements to make use of the TimeEntries that are created in relation to the WorkEffort both in that component and in Manufacturing (as per Jacopo's suggestions).  I suspect this thread or its related JIRA will get more legs once I am closer to delivering something.

[Scott Gray-2]

Just in case you weren't aware, JSR-275 was rejected last month: http://jcp.org/en/jsr/detail?id=275
Not that that means anything, just an FYI.

Regards
Scott

HotWax Media
http://www.hotwaxmedia.com

On 22/04/2010, at 3:15 AM, Adrian Crum wrote:

> Joda Time claims to fix the problems with the Java date/time API, while it duplicates the problems in the Java date/time API. The only real advantage to Joda Time is the additional calendars - which OFBiz already has in the ICU4J library.
>
> The author of JSR-310 complains about Joda in the spec and claims that JSR-310 is better than the Joda Time API. I don't believe JSR-310 will become a part of the Java spec any time soon. JSR-275 is more mature and has seen a lot more activity than JSR-310, yet even that doesn't have enough votes to get it into the Java spec.
>
> If we actually had a choice, I would vote for using JSR-310 over Joda Time. The problem is, there is no library that I can find. So far it appears to be a draft specification only.
>
> In the meantime we could really use a simple set of date/time classes that take the guesswork out of handling dates/times. I started with the Quantity pattern and included some good ideas from JSR-275 and JSR-310. I believe the result is easy to use and reliable.
>
> Mini-language has a set-calendar element - maybe that is the end result of the conversation you were thinking of.
>
> -Adrian
>
> Bob Morley wrote:
>> Adrian Crum-2 wrote:
>>> In the meantime, I'm working on a date/time library that includes an
>>> ElapsedTime class (an improved TimeDuration class) that follows Martin
>>> Fowler's Quantity pattern.
>>>
>> I am certainly not interested in re-inventing the wheel here.  I know in our
>> solution we are going to allow entry against a ProductionRun task in hours
>> and I will want to leverage this TimeDuration as you have in
>> WorkEffortServices.
>> Way back in 2006 - 2007 there was talk of including Joda time into Ofbiz (at
>> the time the thread dealt with wrapping up time related functionality in
>> minilang).  Do you know why we decided to back-off Joda time (if there was a
>> conscious decision there)?  My understanding is that JSR 310 (new java
>> date/time apis) that has been proposed for Java 7 is largely based on Joda
>> time.  Being that the lic. is favorable, I would have thought pulling in and
>> using their Duration might make sense vs. maintaining our own TimeDuration
>> and/or a new date/time library?  Very interested in your thoughts because
>> this really appears to be something in your wheel house.