JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
32 messages Options
12
Reply | Threaded
Open this post in threaded view
|

JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Adrian Crum


Why would you want to do that? The calling method should have a TimeZone and Locale object
available. Any code that uses this method will get unpredictable results.

-Adrian

[hidden email] wrote:

> Author: jaz
> Date: Fri Oct 19 12:09:27 2007
> New Revision: 586582
>
> URL: http://svn.apache.org/viewvc?rev=586582&view=rev
> Log:
> added adjustTimestamp method which doesn't require timezone or locale
>
> Modified:
>     ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
>
> Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java?rev=586582&r1=586581&r2=586582&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java (original)
> +++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java Fri Oct 19 12:09:27 2007
> @@ -771,6 +771,10 @@
>          return new Timestamp(tempCal.getTimeInMillis());
>      }
>  
> +    public static Timestamp adjustTimestamp(Timestamp stamp, int adjType, int adjQuantity) {
> +        return adjustTimestamp(stamp, adjType, adjQuantity, null, null);
> +    }
> +
>      public static Timestamp getDayStart(Timestamp stamp, TimeZone timeZone, Locale locale) {
>          return getDayStart(stamp, 0, timeZone, locale);
>      }
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Tim Ruppert
This probably has to do with that whole backward compatibility thing would be my guess.

Cheers,
Tim
--
Tim Ruppert
HotWax Media

o:801.649.6594
f:801.649.6595

On Oct 25, 2007, at 4:24 PM, Adrian Crum wrote:



From: Adrian Crum <[hidden email]>
Date: October 19, 2007 1:59:54 PM MDT
Subject: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
Reply-To: [hidden email]


Why would you want to do that? The calling method should have a TimeZone and Locale object available. Any code that uses this method will get unpredictable results.

-Adrian


Author: jaz
Date: Fri Oct 19 12:09:27 2007
New Revision: 586582
Log:
added adjustTimestamp method which doesn't require timezone or locale
Modified:
    ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
==============================================================================
--- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java (original)
+++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java Fri Oct 19 12:09:27 2007
@@ -771,6 +771,10 @@
         return new Timestamp(tempCal.getTimeInMillis());
     }
 +    public static Timestamp adjustTimestamp(Timestamp stamp, int adjType, int adjQuantity) {
+        return adjustTimestamp(stamp, adjType, adjQuantity, null, null);
+    }
+
     public static Timestamp getDayStart(Timestamp stamp, TimeZone timeZone, Locale locale) {
         return getDayStart(stamp, 0, timeZone, locale);
     }






smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Adrian Crum
I don't understand what that means. Anyone checking out rev 586582 will have Locale and TimeZone
objects available for the method call. Where is the issue with backward compatibility?

-Adrian

Tim Ruppert wrote:
> This probably has to do with that whole backward compatibility thing
> would be my guess.
>
> Cheers,
> Tim


Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Tim Ruppert
The other method was there before - and was removed.  It's not that there aren't TimeZone and Locale objects available - it's about the large number of installations that have to be upgraded .  That's at least a guess as to why someone would put a method like that in.

Cheers,
Tim
--
Tim Ruppert
HotWax Media

o:801.649.6594
f:801.649.6595


On Oct 25, 2007, at 4:38 PM, Adrian Crum wrote:

I don't understand what that means. Anyone checking out rev 586582 will have Locale and TimeZone objects available for the method call. Where is the issue with backward compatibility?

-Adrian

Tim Ruppert wrote:
This probably has to do with that whole backward compatibility thing would be my guess.
Cheers,
Tim




smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Tim Ruppert
In this case it appears to me that maybe it wasn't backwards compatibility, but just a convenience method.  Not sure - I'll dig around a little more to see use cases.

Cheers,
Tim
--
Tim Ruppert
HotWax Media

o:801.649.6594
f:801.649.6595


On Oct 25, 2007, at 4:41 PM, Tim Ruppert wrote:

The other method was there before - and was removed.  It's not that there aren't TimeZone and Locale objects available - it's about the large number of installations that have to be upgraded .  That's at least a guess as to why someone would put a method like that in.

Cheers,
Tim
--
Tim Ruppert
HotWax Media

o:801.649.6594
f:801.649.6595


On Oct 25, 2007, at 4:38 PM, Adrian Crum wrote:

I don't understand what that means. Anyone checking out rev 586582 will have Locale and TimeZone objects available for the method call. Where is the issue with backward compatibility?

-Adrian

Tim Ruppert wrote:
This probably has to do with that whole backward compatibility thing would be my guess.
Cheers,
Tim





smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Adrian Crum
Tim Ruppert wrote:
> In this case it appears to me that maybe it wasn't backwards
> compatibility, but just a convenience method.  Not sure - I'll dig
> around a little more to see use cases.
>
> Cheers,
> Tim

I created that method as an alternative to doing millisecond arithmetic (which is a big no no). Any
installation at or after rev 564957 will have the objects needed to call the original method.

The Workeffort Calendar is a good example of how to handle date arithmetic properly.

-Adrian


Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Andrew Zeneski-2
In reply to this post by Adrian Crum
The predicted results will be a timestamp adjusted with with the specific amount specified, without a timezone available. Just a helper method.

I used it on a system which is set to GMT and requires dates be displayed in the customer facing in the customer's timezone. The timezone is read as an offset (in javascript) and then adjusted on the fly as needed. There is no TimeZone object available, all we know is how many minutes off GMT.

Andrew

On Oct 25, 2007, at 6:24 PM, Adrian Crum wrote:



From: Adrian Crum <[hidden email]>
Date: October 19, 2007 3:59:54 PM EDT
Subject: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
Reply-To: [hidden email]


Why would you want to do that? The calling method should have a TimeZone and Locale object available. Any code that uses this method will get unpredictable results.

-Adrian


Author: jaz
Date: Fri Oct 19 12:09:27 2007
New Revision: 586582
Log:
added adjustTimestamp method which doesn't require timezone or locale
Modified:
    ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
==============================================================================
--- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java (original)
+++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java Fri Oct 19 12:09:27 2007
@@ -771,6 +771,10 @@
         return new Timestamp(tempCal.getTimeInMillis());
     }
 +    public static Timestamp adjustTimestamp(Timestamp stamp, int adjType, int adjQuantity) {
+        return adjustTimestamp(stamp, adjType, adjQuantity, null, null);
+    }
+
     public static Timestamp getDayStart(Timestamp stamp, TimeZone timeZone, Locale locale) {
         return getDayStart(stamp, 0, timeZone, locale);
     }






smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Andrew Zeneski-2
In reply to this post by Tim Ruppert
No more of a specialized usage which may be useful elsewhere. Its just a static worker method.. :)

On Oct 25, 2007, at 6:34 PM, Tim Ruppert wrote:

This probably has to do with that whole backward compatibility thing would be my guess.

Cheers,
Tim
--
Tim Ruppert
HotWax Media

o:801.649.6594
f:801.649.6595

On Oct 25, 2007, at 4:24 PM, Adrian Crum wrote:



From: Adrian Crum <[hidden email]>
Date: October 19, 2007 1:59:54 PM MDT
Subject: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
Reply-To: [hidden email]


Why would you want to do that? The calling method should have a TimeZone and Locale object available. Any code that uses this method will get unpredictable results.

-Adrian


Author: jaz
Date: Fri Oct 19 12:09:27 2007
New Revision: 586582
Log:
added adjustTimestamp method which doesn't require timezone or locale
Modified:
    ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
Modified: ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java
==============================================================================
--- ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java (original)
+++ ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java Fri Oct 19 12:09:27 2007
@@ -771,6 +771,10 @@
         return new Timestamp(tempCal.getTimeInMillis());
     }
 +    public static Timestamp adjustTimestamp(Timestamp stamp, int adjType, int adjQuantity) {
+        return adjustTimestamp(stamp, adjType, adjQuantity, null, null);
+    }
+
     public static Timestamp getDayStart(Timestamp stamp, TimeZone timeZone, Locale locale) {
         return getDayStart(stamp, 0, timeZone, locale);
     }







smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Adrian Crum
In reply to this post by Andrew Zeneski-2
Andrew Zeneski wrote:

> The predicted results will be a timestamp adjusted with with the
> specific amount specified, without a timezone available. Just a helper
> method.
>
> I used it on a system which is set to GMT and requires dates be
> displayed in the customer facing in the customer's timezone. The
> timezone is read as an offset (in javascript) and then adjusted on the
> fly as needed. There is no TimeZone object available, all we know is how
> many minutes off GMT.
>
> Andrew

The commit log mentions calling this from Freemarker. There are Locale and TimeZone objects in the
Freemarker context. In addition, I'm pretty sure that no matter where you are in the code execution
path, you have access to Locale and TimeZone objects.

-Adrian


Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Andrew Zeneski-2
This method is to use an int offset to adjust the timestamp, without  
a locale or timezone. That's it. No hidden meanings here. It uses  
Integer instead of int since Freemarker doesn't wrap primitives.

On Oct 25, 2007, at 7:05 PM, Adrian Crum wrote:

> Andrew Zeneski wrote:
>> The predicted results will be a timestamp adjusted with with the  
>> specific amount specified, without a timezone available. Just a  
>> helper method.
>> I used it on a system which is set to GMT and requires dates be  
>> displayed in the customer facing in the customer's timezone. The  
>> timezone is read as an offset (in javascript) and then adjusted on  
>> the fly as needed. There is no TimeZone object available, all we  
>> know is how many minutes off GMT.
>> Andrew
>
> The commit log mentions calling this from Freemarker. There are  
> Locale and TimeZone objects in the Freemarker context. In addition,  
> I'm pretty sure that no matter where you are in the code execution  
> path, you have access to Locale and TimeZone objects.
>
> -Adrian
>
>


smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Adrian Crum
Andrew,

I understand what the method does. The point I'm trying to make is this: It is not needed and it
provides a way to introduce inconsistent data into the project.

I understand the method solves a problem for a particular client, but it's not a good thing to
include in the project.

There is a discussion on Jira about this:

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

-Adrian

Andrew Zeneski wrote:

> This method is to use an int offset to adjust the timestamp, without  a
> locale or timezone. That's it. No hidden meanings here. It uses  Integer
> instead of int since Freemarker doesn't wrap primitives.
>
> On Oct 25, 2007, at 7:05 PM, Adrian Crum wrote:
>
>> Andrew Zeneski wrote:
>>
>>> The predicted results will be a timestamp adjusted with with the  
>>> specific amount specified, without a timezone available. Just a  
>>> helper method.
>>> I used it on a system which is set to GMT and requires dates be  
>>> displayed in the customer facing in the customer's timezone. The  
>>> timezone is read as an offset (in javascript) and then adjusted on  
>>> the fly as needed. There is no TimeZone object available, all we  
>>> know is how many minutes off GMT.
>>> Andrew
>>
>>
>> The commit log mentions calling this from Freemarker. There are  
>> Locale and TimeZone objects in the Freemarker context. In addition,  
>> I'm pretty sure that no matter where you are in the code execution  
>> path, you have access to Locale and TimeZone objects.
>>
>> -Adrian
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Andrew Zeneski-2
I disagree. Personally I like methods with optional arguments,  
especially when these arguments aren't really necessary. When you do  
not pass the extra arguments, the assumption should be that you get  
the defaults (default Locale and default TimeZone) which is fine.

In addition, this method was mainly put there for the use of Integer  
instead of int, since the primitive cannot be used inside an FTL.

Andrew

On Oct 25, 2007, at 7:16 PM, Adrian Crum wrote:

> Andrew,
>
> I understand what the method does. The point I'm trying to make is  
> this: It is not needed and it provides a way to introduce  
> inconsistent data into the project.
>
> I understand the method solves a problem for a particular client,  
> but it's not a good thing to include in the project.
>
> There is a discussion on Jira about this:
>
> https://issues.apache.org/jira/browse/OFBIZ-1361
>
> -Adrian
>
> Andrew Zeneski wrote:
>> This method is to use an int offset to adjust the timestamp,  
>> without  a locale or timezone. That's it. No hidden meanings here.  
>> It uses  Integer instead of int since Freemarker doesn't wrap  
>> primitives.
>> On Oct 25, 2007, at 7:05 PM, Adrian Crum wrote:
>>> Andrew Zeneski wrote:
>>>
>>>> The predicted results will be a timestamp adjusted with with  
>>>> the  specific amount specified, without a timezone available.  
>>>> Just a  helper method.
>>>> I used it on a system which is set to GMT and requires dates be  
>>>> displayed in the customer facing in the customer's timezone.  
>>>> The  timezone is read as an offset (in javascript) and then  
>>>> adjusted on  the fly as needed. There is no TimeZone object  
>>>> available, all we  know is how many minutes off GMT.
>>>> Andrew
>>>
>>>
>>> The commit log mentions calling this from Freemarker. There are  
>>> Locale and TimeZone objects in the Freemarker context. In  
>>> addition,  I'm pretty sure that no matter where you are in the  
>>> code execution  path, you have access to Locale and TimeZone  
>>> objects.
>>>
>>> -Adrian
>>>
>>>
>


smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Adrian Crum
Andrew Zeneski wrote:
> I disagree. Personally I like methods with optional arguments,  
> especially when these arguments aren't really necessary. When you do  
> not pass the extra arguments, the assumption should be that you get  the
> defaults (default Locale and default TimeZone) which is fine.

But there is the problem - those arguments ARE really necessary, and getting the defaults is NOT fine.

A properly coded date/time calculation will take into account a user's locale and time zone. If a
developer chooses to use date/time calculations based on defaults, then the user is presented with
two sets of date/time data.

If I add 12 hours to noon today, I would expect the resulting time to be noon tomorrow. Your method
could produce noon tomorrow, or 11 am, or 1 pm - there is no way of knowing the outcome.

Not too long ago the Workeffort calendar was useless - it was inaccurate and buggy. Why? Because it
made the same assumptions that you are making. I fixed the Workeffort calendar by following the
guidelines presented by Sun and IBM - two companies that understand internationalization. My
position in this thread is based upon what I learned while researching the subject, what I learned
from writing my own calendar application, and ultimately taking what I learned and using it to fix
the Workeffort calendar.

-Adrian


Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Adrian Crum
Adrian Crum wrote:
> If I add 12 hours to noon today, I would expect the resulting time to be
> noon tomorrow. Your method could produce noon tomorrow, or 11 am, or 1
> pm - there is no way of knowing the outcome.

lol - of course I meant add 24 hours


Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Tim Ruppert
In reply to this post by Adrian Crum
As an alternative to changing this Adrian, maybe you could just put some javadoc on the method that explains why someone might NOT want to use it. Then you'd get the best of both worlds - both the documentation AND the help.

Cheers,
Tim
--
Tim Ruppert
HotWax Media

o:801.649.6594
f:801.649.6595


On Oct 25, 2007, at 5:47 PM, Adrian Crum wrote:

Andrew Zeneski wrote:
I disagree. Personally I like methods with optional arguments,  especially when these arguments aren't really necessary. When you do  not pass the extra arguments, the assumption should be that you get  the defaults (default Locale and default TimeZone) which is fine.

But there is the problem - those arguments ARE really necessary, and getting the defaults is NOT fine.

A properly coded date/time calculation will take into account a user's locale and time zone. If a developer chooses to use date/time calculations based on defaults, then the user is presented with two sets of date/time data.

If I add 12 hours to noon today, I would expect the resulting time to be noon tomorrow. Your method could produce noon tomorrow, or 11 am, or 1 pm - there is no way of knowing the outcome.

Not too long ago the Workeffort calendar was useless - it was inaccurate and buggy. Why? Because it made the same assumptions that you are making. I fixed the Workeffort calendar by following the guidelines presented by Sun and IBM - two companies that understand internationalization. My position in this thread is based upon what I learned while researching the subject, what I learned from writing my own calendar application, and ultimately taking what I learned and using it to fix the Workeffort calendar.

-Adrian




smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

David E Jones
In reply to this post by Adrian Crum

On Oct 25, 2007, at 5:16 PM, Adrian Crum wrote:

> Andrew,
>
> I understand what the method does. The point I'm trying to make is  
> this: It is not needed and it provides a way to introduce  
> inconsistent data into the project.
>
> I understand the method solves a problem for a particular client,  
> but it's not a good thing to include in the project.
>
> There is a discussion on Jira about this:
>
> https://issues.apache.org/jira/browse/OFBIZ-1361
Yes, and Adrian you seem to have missed my point there, or maybe you  
disagree with me?

The framework is NOT around to force stuff. Here is what I wrote there:

===================================================
While I agree that this should be the best practice, there is a big  
difference in the framework between what we "allow" and what we  
"recommend".

There is lots of stuff you _can_ do with the framework that is really  
not a good idea, thought some might disagree. Things that we  
recommend should be documented in the Best Practices Guide. Other  
things we don't want to make more difficult, IMO, that this is  
important because of the comment about disagreement above. There are  
pretty much always good reasons why we do things the way we recommend  
in the framework, but those recommendations have evolved over time  
and will continue to evolve as well, and not allowing things we don't  
recommend stifles this and limits opportunity to progress and improve.

That is of course a generality, and there is a clear best practice  
here that should be documented and it probably won't ever change, but  
I'm still against forcing on a matter of principle.
===================================================

If you have an issue with that, let's discuss that directly, and if  
necessary vote on it. That seems to be the difference of opinion, so  
let's resolve it... now.

-David


smime.p7s (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Jacques Le Roux
Administrator
And this were comments Adrian, Wickersheimer Jeremy  and I put in this issue

===================================================
Adrian Crum - 21/oct./07 04:15 PM
David,

Thanks for your input. I don't understand what you're saying though.

If those methods aren't deprecated, then developers will continue to use them. This will lead to problems down the road with
inconsistent data - users are going to encounter two different results for the same date/time criteria. Are you saying inconsistent
data is something we should allow?
===================================================

===================================================
Wickersheimer Jeremy - 21/oct./07 07:06 PM
David, i understand your point but marking the methods deprecated doesn't remove them, so they can still be used.
However they should not be used, as Adrian points out they do not use the correct locale so there output will be inconsistent.
Marking the deprecated is a good way to say that the code using them should be migrated at, and it would also make the compiler
throw out useful warnings.
===================================================

===================================================
Jacques Le Roux - 21/oct./07 11:12 PM
David,

I agree with David and Jeremy. Deprecating and documenting it in code seems a good idea in this case. There are better chances to be
read than in the Best practices Guides (pragmatic POV) which does not mean that this should not be documented at this higher level
too. Is there something else we are missing ?
===================================================

So I maybe misunderstood but if deprecating is the way here (you did not say anything about that) why put in the framework a static
method which, according to Adrian and his noon+24h exmaple, is a bad practice ?
Maybe Adrian is wrong about his example but he has done a lot of work around this issue hence it's doubtful.

Mainly  to second Adrian who is really alone trying to explain his view.

Jacques

----- Message d'origine -----
De : "David E Jones" <[hidden email]>
À : <[hidden email]>
Envoyé : vendredi 26 octobre 2007 02:28
Objet : Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]


>
> On Oct 25, 2007, at 5:16 PM, Adrian Crum wrote:
>
> > Andrew,
> >
> > I understand what the method does. The point I'm trying to make is
> > this: It is not needed and it provides a way to introduce
> > inconsistent data into the project.
> >
> > I understand the method solves a problem for a particular client,
> > but it's not a good thing to include in the project.
> >
> > There is a discussion on Jira about this:
> >
> > https://issues.apache.org/jira/browse/OFBIZ-1361
>
> Yes, and Adrian you seem to have missed my point there, or maybe you
> disagree with me?
>
> The framework is NOT around to force stuff. Here is what I wrote there:
>
> ===================================================
> While I agree that this should be the best practice, there is a big
> difference in the framework between what we "allow" and what we
> "recommend".
>
> There is lots of stuff you _can_ do with the framework that is really
> not a good idea, thought some might disagree. Things that we
> recommend should be documented in the Best Practices Guide. Other
> things we don't want to make more difficult, IMO, that this is
> important because of the comment about disagreement above. There are
> pretty much always good reasons why we do things the way we recommend
> in the framework, but those recommendations have evolved over time
> and will continue to evolve as well, and not allowing things we don't
> recommend stifles this and limits opportunity to progress and improve.
>
> That is of course a generality, and there is a clear best practice
> here that should be documented and it probably won't ever change, but
> I'm still against forcing on a matter of principle.
> ===================================================
>
> If you have an issue with that, let's discuss that directly, and if
> necessary vote on it. That seems to be the difference of opinion, so
> let's resolve it... now.
>
> -David
>
>

Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Jacopo Cappellato
Sorry,

I jump in very late in this thread, I'd like to express my point of view
even if I'm missing some of the details and I could be wrong.

I guess that the issue that Andrew faced was related to external
components based on OFBiz: after the OFBiz upgrade they could not
compile because they were using the original datetime methods (not
timezone aware).
If this is true, then maybe Adrian could help by providing an example to
upgrade the old calls to the new ones (for bsh or for ftl etc...); I'm
sure he already did this somewhere, but it is not always easy to track
down where such information is.
In my opinion, adding the method for backward compatibility as Andrew
did it is acceptable too, but I would suggest to deprecate it (and put
an informative message in the deprecation worning to facilitate the
migration) or at least add a warning message in the log: again, Adrian
could help preparing these messages.

Just my 2 cents,

Jacopo


Jacques Le Roux wrote:

> And this were comments Adrian, Wickersheimer Jeremy  and I put in this issue
>
> ===================================================
> Adrian Crum - 21/oct./07 04:15 PM
> David,
>
> Thanks for your input. I don't understand what you're saying though.
>
> If those methods aren't deprecated, then developers will continue to use them. This will lead to problems down the road with
> inconsistent data - users are going to encounter two different results for the same date/time criteria. Are you saying inconsistent
> data is something we should allow?
> ===================================================
>
> ===================================================
> Wickersheimer Jeremy - 21/oct./07 07:06 PM
> David, i understand your point but marking the methods deprecated doesn't remove them, so they can still be used.
> However they should not be used, as Adrian points out they do not use the correct locale so there output will be inconsistent.
> Marking the deprecated is a good way to say that the code using them should be migrated at, and it would also make the compiler
> throw out useful warnings.
> ===================================================
>
> ===================================================
> Jacques Le Roux - 21/oct./07 11:12 PM
> David,
>
> I agree with David and Jeremy. Deprecating and documenting it in code seems a good idea in this case. There are better chances to be
> read than in the Best practices Guides (pragmatic POV) which does not mean that this should not be documented at this higher level
> too. Is there something else we are missing ?
> ===================================================
>
> So I maybe misunderstood but if deprecating is the way here (you did not say anything about that) why put in the framework a static
> method which, according to Adrian and his noon+24h exmaple, is a bad practice ?
> Maybe Adrian is wrong about his example but he has done a lot of work around this issue hence it's doubtful.
>
> Mainly  to second Adrian who is really alone trying to explain his view.
>
> Jacques
>
> ----- Message d'origine -----
> De : "David E Jones" <[hidden email]>
> À : <[hidden email]>
> Envoyé : vendredi 26 octobre 2007 02:28
> Objet : Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]
>
>
>> On Oct 25, 2007, at 5:16 PM, Adrian Crum wrote:
>>
>>> Andrew,
>>>
>>> I understand what the method does. The point I'm trying to make is
>>> this: It is not needed and it provides a way to introduce
>>> inconsistent data into the project.
>>>
>>> I understand the method solves a problem for a particular client,
>>> but it's not a good thing to include in the project.
>>>
>>> There is a discussion on Jira about this:
>>>
>>> https://issues.apache.org/jira/browse/OFBIZ-1361
>> Yes, and Adrian you seem to have missed my point there, or maybe you
>> disagree with me?
>>
>> The framework is NOT around to force stuff. Here is what I wrote there:
>>
>> ===================================================
>> While I agree that this should be the best practice, there is a big
>> difference in the framework between what we "allow" and what we
>> "recommend".
>>
>> There is lots of stuff you _can_ do with the framework that is really
>> not a good idea, thought some might disagree. Things that we
>> recommend should be documented in the Best Practices Guide. Other
>> things we don't want to make more difficult, IMO, that this is
>> important because of the comment about disagreement above. There are
>> pretty much always good reasons why we do things the way we recommend
>> in the framework, but those recommendations have evolved over time
>> and will continue to evolve as well, and not allowing things we don't
>> recommend stifles this and limits opportunity to progress and improve.
>>
>> That is of course a generality, and there is a clear best practice
>> here that should be documented and it probably won't ever change, but
>> I'm still against forcing on a matter of principle.
>> ===================================================
>>
>> If you have an issue with that, let's discuss that directly, and if
>> necessary vote on it. That seems to be the difference of opinion, so
>> let's resolve it... now.
>>
>> -David
>>
>>


Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Scott Gray
Sounds smart to me

Scott

On 26/10/2007, Jacopo Cappellato <[hidden email]> wrote:

>
> Sorry,
>
> I jump in very late in this thread, I'd like to express my point of view
> even if I'm missing some of the details and I could be wrong.
>
> I guess that the issue that Andrew faced was related to external
> components based on OFBiz: after the OFBiz upgrade they could not
> compile because they were using the original datetime methods (not
> timezone aware).
> If this is true, then maybe Adrian could help by providing an example to
> upgrade the old calls to the new ones (for bsh or for ftl etc...); I'm
> sure he already did this somewhere, but it is not always easy to track
> down where such information is.
> In my opinion, adding the method for backward compatibility as Andrew
> did it is acceptable too, but I would suggest to deprecate it (and put
> an informative message in the deprecation worning to facilitate the
> migration) or at least add a warning message in the log: again, Adrian
> could help preparing these messages.
>
> Just my 2 cents,
>
> Jacopo
>
>
> Jacques Le Roux wrote:
> > And this were comments Adrian, Wickersheimer Jeremy  and I put in this
> issue
> >
> > ===================================================
> > Adrian Crum - 21/oct./07 04:15 PM
> > David,
> >
> > Thanks for your input. I don't understand what you're saying though.
> >
> > If those methods aren't deprecated, then developers will continue to use
> them. This will lead to problems down the road with
> > inconsistent data - users are going to encounter two different results
> for the same date/time criteria. Are you saying inconsistent
> > data is something we should allow?
> > ===================================================
> >
> > ===================================================
> > Wickersheimer Jeremy - 21/oct./07 07:06 PM
> > David, i understand your point but marking the methods deprecated
> doesn't remove them, so they can still be used.
> > However they should not be used, as Adrian points out they do not use
> the correct locale so there output will be inconsistent.
> > Marking the deprecated is a good way to say that the code using them
> should be migrated at, and it would also make the compiler
> > throw out useful warnings.
> > ===================================================
> >
> > ===================================================
> > Jacques Le Roux - 21/oct./07 11:12 PM
> > David,
> >
> > I agree with David and Jeremy. Deprecating and documenting it in code
> seems a good idea in this case. There are better chances to be
> > read than in the Best practices Guides (pragmatic POV) which does not
> mean that this should not be documented at this higher level
> > too. Is there something else we are missing ?
> > ===================================================
> >
> > So I maybe misunderstood but if deprecating is the way here (you did not
> say anything about that) why put in the framework a static
> > method which, according to Adrian and his noon+24h exmaple, is a bad
> practice ?
> > Maybe Adrian is wrong about his example but he has done a lot of work
> around this issue hence it's doubtful.
> >
> > Mainly  to second Adrian who is really alone trying to explain his view.
> >
> > Jacques
> >
> > ----- Message d'origine -----
> > De : "David E Jones" <[hidden email]>
> > À : <[hidden email]>
> > Envoyé : vendredi 26 octobre 2007 02:28
> > Objet : Re: JAZ- [Fwd: Re: svn commit: r586582 -
> /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]
> >
> >
> >> On Oct 25, 2007, at 5:16 PM, Adrian Crum wrote:
> >>
> >>> Andrew,
> >>>
> >>> I understand what the method does. The point I'm trying to make is
> >>> this: It is not needed and it provides a way to introduce
> >>> inconsistent data into the project.
> >>>
> >>> I understand the method solves a problem for a particular client,
> >>> but it's not a good thing to include in the project.
> >>>
> >>> There is a discussion on Jira about this:
> >>>
> >>> https://issues.apache.org/jira/browse/OFBIZ-1361
> >> Yes, and Adrian you seem to have missed my point there, or maybe you
> >> disagree with me?
> >>
> >> The framework is NOT around to force stuff. Here is what I wrote there:
> >>
> >> ===================================================
> >> While I agree that this should be the best practice, there is a big
> >> difference in the framework between what we "allow" and what we
> >> "recommend".
> >>
> >> There is lots of stuff you _can_ do with the framework that is really
> >> not a good idea, thought some might disagree. Things that we
> >> recommend should be documented in the Best Practices Guide. Other
> >> things we don't want to make more difficult, IMO, that this is
> >> important because of the comment about disagreement above. There are
> >> pretty much always good reasons why we do things the way we recommend
> >> in the framework, but those recommendations have evolved over time
> >> and will continue to evolve as well, and not allowing things we don't
> >> recommend stifles this and limits opportunity to progress and improve.
> >>
> >> That is of course a generality, and there is a clear best practice
> >> here that should be documented and it probably won't ever change, but
> >> I'm still against forcing on a matter of principle.
> >> ===================================================
> >>
> >> If you have an issue with that, let's discuss that directly, and if
> >> necessary vote on it. That seems to be the difference of opinion, so
> >> let's resolve it... now.
> >>
> >> -David
> >>
> >>
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: JAZ- [Fwd: Re: svn commit: r586582 - /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]

Jacques Le Roux
Administrator
Yes good consensus

Jacques

De : "Scott Gray" <[hidden email]>
Sounds smart to me

Scott

On 26/10/2007, Jacopo Cappellato <[hidden email]> wrote:

>
> Sorry,
>
> I jump in very late in this thread, I'd like to express my point of view
> even if I'm missing some of the details and I could be wrong.
>
> I guess that the issue that Andrew faced was related to external
> components based on OFBiz: after the OFBiz upgrade they could not
> compile because they were using the original datetime methods (not
> timezone aware).
> If this is true, then maybe Adrian could help by providing an example to
> upgrade the old calls to the new ones (for bsh or for ftl etc...); I'm
> sure he already did this somewhere, but it is not always easy to track
> down where such information is.
> In my opinion, adding the method for backward compatibility as Andrew
> did it is acceptable too, but I would suggest to deprecate it (and put
> an informative message in the deprecation worning to facilitate the
> migration) or at least add a warning message in the log: again, Adrian
> could help preparing these messages.
>
> Just my 2 cents,
>
> Jacopo
>
>
> Jacques Le Roux wrote:
> > And this were comments Adrian, Wickersheimer Jeremy  and I put in this
> issue
> >
> > ===================================================
> > Adrian Crum - 21/oct./07 04:15 PM
> > David,
> >
> > Thanks for your input. I don't understand what you're saying though.
> >
> > If those methods aren't deprecated, then developers will continue to use
> them. This will lead to problems down the road with
> > inconsistent data - users are going to encounter two different results
> for the same date/time criteria. Are you saying inconsistent
> > data is something we should allow?
> > ===================================================
> >
> > ===================================================
> > Wickersheimer Jeremy - 21/oct./07 07:06 PM
> > David, i understand your point but marking the methods deprecated
> doesn't remove them, so they can still be used.
> > However they should not be used, as Adrian points out they do not use
> the correct locale so there output will be inconsistent.
> > Marking the deprecated is a good way to say that the code using them
> should be migrated at, and it would also make the compiler
> > throw out useful warnings.
> > ===================================================
> >
> > ===================================================
> > Jacques Le Roux - 21/oct./07 11:12 PM
> > David,
> >
> > I agree with David and Jeremy. Deprecating and documenting it in code
> seems a good idea in this case. There are better chances to be
> > read than in the Best practices Guides (pragmatic POV) which does not
> mean that this should not be documented at this higher level
> > too. Is there something else we are missing ?
> > ===================================================
> >
> > So I maybe misunderstood but if deprecating is the way here (you did not
> say anything about that) why put in the framework a static
> > method which, according to Adrian and his noon+24h exmaple, is a bad
> practice ?
> > Maybe Adrian is wrong about his example but he has done a lot of work
> around this issue hence it's doubtful.
> >
> > Mainly  to second Adrian who is really alone trying to explain his view.
> >
> > Jacques
> >
> > ----- Message d'origine -----
> > De : "David E Jones" <[hidden email]>
> > À : <[hidden email]>
> > Envoyé : vendredi 26 octobre 2007 02:28
> > Objet : Re: JAZ- [Fwd: Re: svn commit: r586582 -
> /ofbiz/trunk/framework/base/src/base/org/ofbiz/base/util/UtilDateTime.java]
> >
> >
> >> On Oct 25, 2007, at 5:16 PM, Adrian Crum wrote:
> >>
> >>> Andrew,
> >>>
> >>> I understand what the method does. The point I'm trying to make is
> >>> this: It is not needed and it provides a way to introduce
> >>> inconsistent data into the project.
> >>>
> >>> I understand the method solves a problem for a particular client,
> >>> but it's not a good thing to include in the project.
> >>>
> >>> There is a discussion on Jira about this:
> >>>
> >>> https://issues.apache.org/jira/browse/OFBIZ-1361
> >> Yes, and Adrian you seem to have missed my point there, or maybe you
> >> disagree with me?
> >>
> >> The framework is NOT around to force stuff. Here is what I wrote there:
> >>
> >> ===================================================
> >> While I agree that this should be the best practice, there is a big
> >> difference in the framework between what we "allow" and what we
> >> "recommend".
> >>
> >> There is lots of stuff you _can_ do with the framework that is really
> >> not a good idea, thought some might disagree. Things that we
> >> recommend should be documented in the Best Practices Guide. Other
> >> things we don't want to make more difficult, IMO, that this is
> >> important because of the comment about disagreement above. There are
> >> pretty much always good reasons why we do things the way we recommend
> >> in the framework, but those recommendations have evolved over time
> >> and will continue to evolve as well, and not allowing things we don't
> >> recommend stifles this and limits opportunity to progress and improve.
> >>
> >> That is of course a generality, and there is a clear best practice
> >> here that should be documented and it probably won't ever change, but
> >> I'm still against forcing on a matter of principle.
> >> ===================================================
> >>
> >> If you have an issue with that, let's discuss that directly, and if
> >> necessary vote on it. That seems to be the difference of opinion, so
> >> let's resolve it... now.
> >>
> >> -David
> >>
> >>
>
>
>

12