Re: svn commit: r943843 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

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

Re: svn commit: r943843 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

Adrian Crum
Just an observation, not commenting on this commit specifically:

The conversion to String should use the
ObjectType.simpleTypeConvert(...) method - there are a lot of
conversions being missed in this code.

-Adrian

On 5/13/2010 1:32 AM, [hidden email] wrote:

> Author: jonesde
> Date: Thu May 13 08:32:09 2010
> New Revision: 943843
>
> URL: http://svn.apache.org/viewvc?rev=943843&view=rev
> Log:
> Made widget link parameter treatment more consistent; before if you specified a paramaeter name without a from-field then it wouldn't convert the object according to locale; now it does the same thing no matter where the object comes from (ie explicit from-field or implied by the parameter name); this fixes a timeZone inconsistency that causes links with time/timestamp parameters to not work
>
> Modified:
>      ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>
> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=943843&r1=943842&r2=943843&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java (original)
> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java Thu May 13 08:32:09 2010
> @@ -30,6 +30,7 @@ import javax.servlet.ServletContext;
>   import javax.servlet.http.HttpServletRequest;
>   import javax.servlet.http.HttpServletResponse;
>
> +import org.ofbiz.base.util.Debug;
>   import org.ofbiz.base.util.StringUtil;
>   import org.ofbiz.base.util.UtilDateTime;
>   import org.ofbiz.base.util.UtilGenerics;
> @@ -316,44 +317,41 @@ public class WidgetWorker {
>           public String getValue(Map<String, Object>  context) {
>               if (this.value != null) {
>                   return this.value.expandString(context);
> -            } else if (this.fromField != null&&  this.fromField.get(context) != null) {
> -                Object retVal = this.fromField.get(context);
> +            }
> +
> +            Object retVal = null;
> +            if (this.fromField != null&&  this.fromField.get(context) != null) {
> +                retVal = this.fromField.get(context);
> +            } else {
> +                retVal = context.get(this.name);
> +            }
> +
> +            if (retVal != null) {
> +                TimeZone timeZone = (TimeZone) context.get("timeZone");
> +                if (timeZone == null) timeZone = TimeZone.getDefault();
>
> -                if (retVal != null) {
> -                    TimeZone timeZone = (TimeZone) context.get("timeZone");
> -                    if (timeZone == null) timeZone = TimeZone.getDefault();
> -
> -                    String returnValue = null;
> -                    // format string based on the user's time zone (not locale because these are parameters)
> -                    if (retVal instanceof Double || retVal instanceof Float || retVal instanceof BigDecimal) {
> -                        returnValue = retVal.toString();
> -                    } else if (retVal instanceof java.sql.Date) {
> -                        DateFormat df = UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT, timeZone, null);
> -                        returnValue = df.format((java.util.Date) retVal);
> -                    } else if (retVal instanceof java.sql.Time) {
> -                        DateFormat df = UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT, timeZone, null);
> -                        returnValue = df.format((java.util.Date) retVal);
> -                    } else if (retVal instanceof java.sql.Timestamp) {
> -                        DateFormat df = UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT, timeZone, null);
> -                        returnValue = df.format((java.util.Date) retVal);
> -                    } else if (retVal instanceof java.util.Date) {
> -                        DateFormat df = UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy", timeZone, null);
> -                        returnValue = df.format((java.util.Date) retVal);
> -                    } else {
> -                        returnValue = retVal.toString();
> -                    }
> -                    return returnValue;
> +                String returnValue = null;
> +                // format string based on the user's time zone (not locale because these are parameters)
> +                if (retVal instanceof Double || retVal instanceof Float || retVal instanceof BigDecimal) {
> +                    returnValue = retVal.toString();
> +                } else if (retVal instanceof java.sql.Date) {
> +                    DateFormat df = UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT, timeZone, null);
> +                    returnValue = df.format((java.util.Date) retVal);
> +                } else if (retVal instanceof java.sql.Time) {
> +                    DateFormat df = UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT, timeZone, null);
> +                    returnValue = df.format((java.util.Date) retVal);
> +                } else if (retVal instanceof java.sql.Timestamp) {
> +                    DateFormat df = UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT, timeZone, null);
> +                    returnValue = df.format((java.util.Date) retVal);
> +                } else if (retVal instanceof java.util.Date) {
> +                    DateFormat df = UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy", timeZone, null);
> +                    returnValue = df.format((java.util.Date) retVal);
>                   } else {
> -                    return null;
> +                    returnValue = retVal.toString();
>                   }
> +                return returnValue;
>               } else {
> -                // as a last chance try finding a context field with the key of the name field
> -                Object obj = context.get(this.name);
> -                if (obj != null) {
> -                    return obj.toString();
> -                } else {
> -                    return null;
> -                }
> +                return null;
>               }
>           }
>       }
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r943843 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

David E. Jones-2

Which brings up some questions:

1. is there an "official" way to convert types that should be used everywhere (I think that's what you're saying)?
2. how many things are not using that?
3. does the official way handle Object -> String conversions for human versus machine (ie HTTP parameter) use?

-David


On May 13, 2010, at 8:54 AM, Adrian Crum wrote:

> Just an observation, not commenting on this commit specifically:
>
> The conversion to String should use the ObjectType.simpleTypeConvert(...) method - there are a lot of conversions being missed in this code.
>
> -Adrian
>
> On 5/13/2010 1:32 AM, [hidden email] wrote:
>> Author: jonesde
>> Date: Thu May 13 08:32:09 2010
>> New Revision: 943843
>>
>> URL: http://svn.apache.org/viewvc?rev=943843&view=rev
>> Log:
>> Made widget link parameter treatment more consistent; before if you specified a paramaeter name without a from-field then it wouldn't convert the object according to locale; now it does the same thing no matter where the object comes from (ie explicit from-field or implied by the parameter name); this fixes a timeZone inconsistency that causes links with time/timestamp parameters to not work
>>
>> Modified:
>>     ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>>
>> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=943843&r1=943842&r2=943843&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java (original)
>> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java Thu May 13 08:32:09 2010
>> @@ -30,6 +30,7 @@ import javax.servlet.ServletContext;
>>  import javax.servlet.http.HttpServletRequest;
>>  import javax.servlet.http.HttpServletResponse;
>>
>> +import org.ofbiz.base.util.Debug;
>>  import org.ofbiz.base.util.StringUtil;
>>  import org.ofbiz.base.util.UtilDateTime;
>>  import org.ofbiz.base.util.UtilGenerics;
>> @@ -316,44 +317,41 @@ public class WidgetWorker {
>>          public String getValue(Map<String, Object>  context) {
>>              if (this.value != null) {
>>                  return this.value.expandString(context);
>> -            } else if (this.fromField != null&&  this.fromField.get(context) != null) {
>> -                Object retVal = this.fromField.get(context);
>> +            }
>> +
>> +            Object retVal = null;
>> +            if (this.fromField != null&&  this.fromField.get(context) != null) {
>> +                retVal = this.fromField.get(context);
>> +            } else {
>> +                retVal = context.get(this.name);
>> +            }
>> +
>> +            if (retVal != null) {
>> +                TimeZone timeZone = (TimeZone) context.get("timeZone");
>> +                if (timeZone == null) timeZone = TimeZone.getDefault();
>>
>> -                if (retVal != null) {
>> -                    TimeZone timeZone = (TimeZone) context.get("timeZone");
>> -                    if (timeZone == null) timeZone = TimeZone.getDefault();
>> -
>> -                    String returnValue = null;
>> -                    // format string based on the user's time zone (not locale because these are parameters)
>> -                    if (retVal instanceof Double || retVal instanceof Float || retVal instanceof BigDecimal) {
>> -                        returnValue = retVal.toString();
>> -                    } else if (retVal instanceof java.sql.Date) {
>> -                        DateFormat df = UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT, timeZone, null);
>> -                        returnValue = df.format((java.util.Date) retVal);
>> -                    } else if (retVal instanceof java.sql.Time) {
>> -                        DateFormat df = UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT, timeZone, null);
>> -                        returnValue = df.format((java.util.Date) retVal);
>> -                    } else if (retVal instanceof java.sql.Timestamp) {
>> -                        DateFormat df = UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT, timeZone, null);
>> -                        returnValue = df.format((java.util.Date) retVal);
>> -                    } else if (retVal instanceof java.util.Date) {
>> -                        DateFormat df = UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy", timeZone, null);
>> -                        returnValue = df.format((java.util.Date) retVal);
>> -                    } else {
>> -                        returnValue = retVal.toString();
>> -                    }
>> -                    return returnValue;
>> +                String returnValue = null;
>> +                // format string based on the user's time zone (not locale because these are parameters)
>> +                if (retVal instanceof Double || retVal instanceof Float || retVal instanceof BigDecimal) {
>> +                    returnValue = retVal.toString();
>> +                } else if (retVal instanceof java.sql.Date) {
>> +                    DateFormat df = UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT, timeZone, null);
>> +                    returnValue = df.format((java.util.Date) retVal);
>> +                } else if (retVal instanceof java.sql.Time) {
>> +                    DateFormat df = UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT, timeZone, null);
>> +                    returnValue = df.format((java.util.Date) retVal);
>> +                } else if (retVal instanceof java.sql.Timestamp) {
>> +                    DateFormat df = UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT, timeZone, null);
>> +                    returnValue = df.format((java.util.Date) retVal);
>> +                } else if (retVal instanceof java.util.Date) {
>> +                    DateFormat df = UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy", timeZone, null);
>> +                    returnValue = df.format((java.util.Date) retVal);
>>                  } else {
>> -                    return null;
>> +                    returnValue = retVal.toString();
>>                  }
>> +                return returnValue;
>>              } else {
>> -                // as a last chance try finding a context field with the key of the name field
>> -                Object obj = context.get(this.name);
>> -                if (obj != null) {
>> -                    return obj.toString();
>> -                } else {
>> -                    return null;
>> -                }
>> +                return null;
>>              }
>>          }
>>      }
>>
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r943843 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

Adrian Crum
1. I don't know that it is official - it's more common sense: If code
exists to convert an object from one type to another, why write more
code that does the same thing?

2. I don't know. We would have to look for similar code.

3. The best thing to do would be to use stronger data typing. An HTTP
(or URL) parameter should be a type - instead of using a String. If we
had a type for that, then all kinds of convenient behavior could be
attached to it (like encoding/decoding, etc).

-Adrian

On 5/13/2010 9:09 AM, David E Jones wrote:

>
> Which brings up some questions:
>
> 1. is there an "official" way to convert types that should be used everywhere (I think that's what you're saying)?
> 2. how many things are not using that?
> 3. does the official way handle Object ->  String conversions for human versus machine (ie HTTP parameter) use?
>
> -David
>
>
> On May 13, 2010, at 8:54 AM, Adrian Crum wrote:
>
>> Just an observation, not commenting on this commit specifically:
>>
>> The conversion to String should use the ObjectType.simpleTypeConvert(...) method - there are a lot of conversions being missed in this code.
>>
>> -Adrian
>>
>> On 5/13/2010 1:32 AM, [hidden email] wrote:
>>> Author: jonesde
>>> Date: Thu May 13 08:32:09 2010
>>> New Revision: 943843
>>>
>>> URL: http://svn.apache.org/viewvc?rev=943843&view=rev
>>> Log:
>>> Made widget link parameter treatment more consistent; before if you specified a paramaeter name without a from-field then it wouldn't convert the object according to locale; now it does the same thing no matter where the object comes from (ie explicit from-field or implied by the parameter name); this fixes a timeZone inconsistency that causes links with time/timestamp parameters to not work
>>>
>>> Modified:
>>>      ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>>>
>>> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=943843&r1=943842&r2=943843&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java (original)
>>> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java Thu May 13 08:32:09 2010
>>> @@ -30,6 +30,7 @@ import javax.servlet.ServletContext;
>>>   import javax.servlet.http.HttpServletRequest;
>>>   import javax.servlet.http.HttpServletResponse;
>>>
>>> +import org.ofbiz.base.util.Debug;
>>>   import org.ofbiz.base.util.StringUtil;
>>>   import org.ofbiz.base.util.UtilDateTime;
>>>   import org.ofbiz.base.util.UtilGenerics;
>>> @@ -316,44 +317,41 @@ public class WidgetWorker {
>>>           public String getValue(Map<String, Object>   context) {
>>>               if (this.value != null) {
>>>                   return this.value.expandString(context);
>>> -            } else if (this.fromField != null&&   this.fromField.get(context) != null) {
>>> -                Object retVal = this.fromField.get(context);
>>> +            }
>>> +
>>> +            Object retVal = null;
>>> +            if (this.fromField != null&&   this.fromField.get(context) != null) {
>>> +                retVal = this.fromField.get(context);
>>> +            } else {
>>> +                retVal = context.get(this.name);
>>> +            }
>>> +
>>> +            if (retVal != null) {
>>> +                TimeZone timeZone = (TimeZone) context.get("timeZone");
>>> +                if (timeZone == null) timeZone = TimeZone.getDefault();
>>>
>>> -                if (retVal != null) {
>>> -                    TimeZone timeZone = (TimeZone) context.get("timeZone");
>>> -                    if (timeZone == null) timeZone = TimeZone.getDefault();
>>> -
>>> -                    String returnValue = null;
>>> -                    // format string based on the user's time zone (not locale because these are parameters)
>>> -                    if (retVal instanceof Double || retVal instanceof Float || retVal instanceof BigDecimal) {
>>> -                        returnValue = retVal.toString();
>>> -                    } else if (retVal instanceof java.sql.Date) {
>>> -                        DateFormat df = UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT, timeZone, null);
>>> -                        returnValue = df.format((java.util.Date) retVal);
>>> -                    } else if (retVal instanceof java.sql.Time) {
>>> -                        DateFormat df = UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT, timeZone, null);
>>> -                        returnValue = df.format((java.util.Date) retVal);
>>> -                    } else if (retVal instanceof java.sql.Timestamp) {
>>> -                        DateFormat df = UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT, timeZone, null);
>>> -                        returnValue = df.format((java.util.Date) retVal);
>>> -                    } else if (retVal instanceof java.util.Date) {
>>> -                        DateFormat df = UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy", timeZone, null);
>>> -                        returnValue = df.format((java.util.Date) retVal);
>>> -                    } else {
>>> -                        returnValue = retVal.toString();
>>> -                    }
>>> -                    return returnValue;
>>> +                String returnValue = null;
>>> +                // format string based on the user's time zone (not locale because these are parameters)
>>> +                if (retVal instanceof Double || retVal instanceof Float || retVal instanceof BigDecimal) {
>>> +                    returnValue = retVal.toString();
>>> +                } else if (retVal instanceof java.sql.Date) {
>>> +                    DateFormat df = UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT, timeZone, null);
>>> +                    returnValue = df.format((java.util.Date) retVal);
>>> +                } else if (retVal instanceof java.sql.Time) {
>>> +                    DateFormat df = UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT, timeZone, null);
>>> +                    returnValue = df.format((java.util.Date) retVal);
>>> +                } else if (retVal instanceof java.sql.Timestamp) {
>>> +                    DateFormat df = UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT, timeZone, null);
>>> +                    returnValue = df.format((java.util.Date) retVal);
>>> +                } else if (retVal instanceof java.util.Date) {
>>> +                    DateFormat df = UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy", timeZone, null);
>>> +                    returnValue = df.format((java.util.Date) retVal);
>>>                   } else {
>>> -                    return null;
>>> +                    returnValue = retVal.toString();
>>>                   }
>>> +                return returnValue;
>>>               } else {
>>> -                // as a last chance try finding a context field with the key of the name field
>>> -                Object obj = context.get(this.name);
>>> -                if (obj != null) {
>>> -                    return obj.toString();
>>> -                } else {
>>> -                    return null;
>>> -                }
>>> +                return null;
>>>               }
>>>           }
>>>       }
>>>
>>>
>>>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r943843 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

David E. Jones-2

Sorry, I meant for those questions to be an aspect of rhetoric instead of trying to write out some sort of requirement idea or issue.

Whatever the case, your answer #3 is interesting... why use a custom type with encoding/decoding when the current SQL Date, Time, and Timestamp do so just fine (as long as you use it's default toString and valueOf methods, with TimeZone adjustment because the server will do that because we don't distinguish between user input and system generated text). It might actually be helpful to change how system generated object Strings are done, ie those not meant for human consumption, and for dates/times we could use an absolute measure (like the millis from epoch) so that we don't have to know which timezone the user is in to find the right record in the database.

I'm not sure if that's a good idea, but thought I'd throw it out there...

-David


On May 13, 2010, at 10:18 AM, Adrian Crum wrote:

> 1. I don't know that it is official - it's more common sense: If code exists to convert an object from one type to another, why write more code that does the same thing?
>
> 2. I don't know. We would have to look for similar code.
>
> 3. The best thing to do would be to use stronger data typing. An HTTP (or URL) parameter should be a type - instead of using a String. If we had a type for that, then all kinds of convenient behavior could be attached to it (like encoding/decoding, etc).
>
> -Adrian
>
> On 5/13/2010 9:09 AM, David E Jones wrote:
>>
>> Which brings up some questions:
>>
>> 1. is there an "official" way to convert types that should be used everywhere (I think that's what you're saying)?
>> 2. how many things are not using that?
>> 3. does the official way handle Object ->  String conversions for human versus machine (ie HTTP parameter) use?
>>
>> -David
>>
>>
>> On May 13, 2010, at 8:54 AM, Adrian Crum wrote:
>>
>>> Just an observation, not commenting on this commit specifically:
>>>
>>> The conversion to String should use the ObjectType.simpleTypeConvert(...) method - there are a lot of conversions being missed in this code.
>>>
>>> -Adrian
>>>
>>> On 5/13/2010 1:32 AM, [hidden email] wrote:
>>>> Author: jonesde
>>>> Date: Thu May 13 08:32:09 2010
>>>> New Revision: 943843
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=943843&view=rev
>>>> Log:
>>>> Made widget link parameter treatment more consistent; before if you specified a paramaeter name without a from-field then it wouldn't convert the object according to locale; now it does the same thing no matter where the object comes from (ie explicit from-field or implied by the parameter name); this fixes a timeZone inconsistency that causes links with time/timestamp parameters to not work
>>>>
>>>> Modified:
>>>>     ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>>>>
>>>> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=943843&r1=943842&r2=943843&view=diff
>>>> ==============================================================================
>>>> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java (original)
>>>> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java Thu May 13 08:32:09 2010
>>>> @@ -30,6 +30,7 @@ import javax.servlet.ServletContext;
>>>>  import javax.servlet.http.HttpServletRequest;
>>>>  import javax.servlet.http.HttpServletResponse;
>>>>
>>>> +import org.ofbiz.base.util.Debug;
>>>>  import org.ofbiz.base.util.StringUtil;
>>>>  import org.ofbiz.base.util.UtilDateTime;
>>>>  import org.ofbiz.base.util.UtilGenerics;
>>>> @@ -316,44 +317,41 @@ public class WidgetWorker {
>>>>          public String getValue(Map<String, Object>   context) {
>>>>              if (this.value != null) {
>>>>                  return this.value.expandString(context);
>>>> -            } else if (this.fromField != null&&   this.fromField.get(context) != null) {
>>>> -                Object retVal = this.fromField.get(context);
>>>> +            }
>>>> +
>>>> +            Object retVal = null;
>>>> +            if (this.fromField != null&&   this.fromField.get(context) != null) {
>>>> +                retVal = this.fromField.get(context);
>>>> +            } else {
>>>> +                retVal = context.get(this.name);
>>>> +            }
>>>> +
>>>> +            if (retVal != null) {
>>>> +                TimeZone timeZone = (TimeZone) context.get("timeZone");
>>>> +                if (timeZone == null) timeZone = TimeZone.getDefault();
>>>>
>>>> -                if (retVal != null) {
>>>> -                    TimeZone timeZone = (TimeZone) context.get("timeZone");
>>>> -                    if (timeZone == null) timeZone = TimeZone.getDefault();
>>>> -
>>>> -                    String returnValue = null;
>>>> -                    // format string based on the user's time zone (not locale because these are parameters)
>>>> -                    if (retVal instanceof Double || retVal instanceof Float || retVal instanceof BigDecimal) {
>>>> -                        returnValue = retVal.toString();
>>>> -                    } else if (retVal instanceof java.sql.Date) {
>>>> -                        DateFormat df = UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT, timeZone, null);
>>>> -                        returnValue = df.format((java.util.Date) retVal);
>>>> -                    } else if (retVal instanceof java.sql.Time) {
>>>> -                        DateFormat df = UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT, timeZone, null);
>>>> -                        returnValue = df.format((java.util.Date) retVal);
>>>> -                    } else if (retVal instanceof java.sql.Timestamp) {
>>>> -                        DateFormat df = UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT, timeZone, null);
>>>> -                        returnValue = df.format((java.util.Date) retVal);
>>>> -                    } else if (retVal instanceof java.util.Date) {
>>>> -                        DateFormat df = UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy", timeZone, null);
>>>> -                        returnValue = df.format((java.util.Date) retVal);
>>>> -                    } else {
>>>> -                        returnValue = retVal.toString();
>>>> -                    }
>>>> -                    return returnValue;
>>>> +                String returnValue = null;
>>>> +                // format string based on the user's time zone (not locale because these are parameters)
>>>> +                if (retVal instanceof Double || retVal instanceof Float || retVal instanceof BigDecimal) {
>>>> +                    returnValue = retVal.toString();
>>>> +                } else if (retVal instanceof java.sql.Date) {
>>>> +                    DateFormat df = UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT, timeZone, null);
>>>> +                    returnValue = df.format((java.util.Date) retVal);
>>>> +                } else if (retVal instanceof java.sql.Time) {
>>>> +                    DateFormat df = UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT, timeZone, null);
>>>> +                    returnValue = df.format((java.util.Date) retVal);
>>>> +                } else if (retVal instanceof java.sql.Timestamp) {
>>>> +                    DateFormat df = UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT, timeZone, null);
>>>> +                    returnValue = df.format((java.util.Date) retVal);
>>>> +                } else if (retVal instanceof java.util.Date) {
>>>> +                    DateFormat df = UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy", timeZone, null);
>>>> +                    returnValue = df.format((java.util.Date) retVal);
>>>>                  } else {
>>>> -                    return null;
>>>> +                    returnValue = retVal.toString();
>>>>                  }
>>>> +                return returnValue;
>>>>              } else {
>>>> -                // as a last chance try finding a context field with the key of the name field
>>>> -                Object obj = context.get(this.name);
>>>> -                if (obj != null) {
>>>> -                    return obj.toString();
>>>> -                } else {
>>>> -                    return null;
>>>> -                }
>>>> +                return null;
>>>>              }
>>>>          }
>>>>      }
>>>>
>>>>
>>>>
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r943843 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

Adrian Crum-2
Using the long value for date/time parameters is probably the most reliable way to do it. The Work Effort calendar does it that way for some URL parameters.

Why use a custom type for parameters? You answered that question yourself:

"It might actually be helpful to change how system generated object Strings are done, ie those not meant for human consumption..."

Somewhere in the execution path there needs to be a way to flag data types as being intended to be used as parameters. My suggestion is to encapsulate the data in a Parameter data type, it sounds like your suggestion is to flag data types in some way or have data types destined for parameters be routed to some other execution path.

I think it boils down to two different ways of looking at the problem.

-Adrian

--- On Thu, 5/13/10, David E Jones <[hidden email]> wrote:

> Sorry, I meant for those questions to be an aspect of
> rhetoric instead of trying to write out some sort of
> requirement idea or issue.
>
> Whatever the case, your answer #3 is interesting... why use
> a custom type with encoding/decoding when the current SQL
> Date, Time, and Timestamp do so just fine (as long as you
> use it's default toString and valueOf methods, with TimeZone
> adjustment because the server will do that because we don't
> distinguish between user input and system generated text).
> It might actually be helpful to change how system generated
> object Strings are done, ie those not meant for human
> consumption, and for dates/times we could use an absolute
> measure (like the millis from epoch) so that we don't have
> to know which timezone the user is in to find the right
> record in the database.
>
> I'm not sure if that's a good idea, but thought I'd throw
> it out there...
>
> -David
>
>
> On May 13, 2010, at 10:18 AM, Adrian Crum wrote:
>
> > 1. I don't know that it is official - it's more common
> sense: If code exists to convert an object from one type to
> another, why write more code that does the same thing?
> >
> > 2. I don't know. We would have to look for similar
> code.
> >
> > 3. The best thing to do would be to use stronger data
> typing. An HTTP (or URL) parameter should be a type -
> instead of using a String. If we had a type for that, then
> all kinds of convenient behavior could be attached to it
> (like encoding/decoding, etc).
> >
> > -Adrian
> >
> > On 5/13/2010 9:09 AM, David E Jones wrote:
> >>
> >> Which brings up some questions:
> >>
> >> 1. is there an "official" way to convert types
> that should be used everywhere (I think that's what you're
> saying)?
> >> 2. how many things are not using that?
> >> 3. does the official way handle Object -> 
> String conversions for human versus machine (ie HTTP
> parameter) use?
> >>
> >> -David
> >>
> >>
> >> On May 13, 2010, at 8:54 AM, Adrian Crum wrote:
> >>
> >>> Just an observation, not commenting on this
> commit specifically:
> >>>
> >>> The conversion to String should use the
> ObjectType.simpleTypeConvert(...) method - there are a lot
> of conversions being missed in this code.
> >>>
> >>> -Adrian
> >>>
> >>> On 5/13/2010 1:32 AM, [hidden email]
> wrote:
> >>>> Author: jonesde
> >>>> Date: Thu May 13 08:32:09 2010
> >>>> New Revision: 943843
> >>>>
> >>>> URL: http://svn.apache.org/viewvc?rev=943843&view=rev
> >>>> Log:
> >>>> Made widget link parameter treatment more
> consistent; before if you specified a paramaeter name
> without a from-field then it wouldn't convert the object
> according to locale; now it does the same thing no matter
> where the object comes from (ie explicit from-field or
> implied by the parameter name); this fixes a timeZone
> inconsistency that causes links with time/timestamp
> parameters to not work
> >>>>
> >>>> Modified:
> >>>> 
>    ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> >>>>
> >>>> Modified:
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> >>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=943843&r1=943842&r2=943843&view=diff
> >>>>
> ==============================================================================
> >>>> ---
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> (original)
> >>>> +++
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> Thu May 13 08:32:09 2010
> >>>> @@ -30,6 +30,7 @@ import
> javax.servlet.ServletContext;
> >>>>  import
> javax.servlet.http.HttpServletRequest;
> >>>>  import
> javax.servlet.http.HttpServletResponse;
> >>>>
> >>>> +import org.ofbiz.base.util.Debug;
> >>>>  import
> org.ofbiz.base.util.StringUtil;
> >>>>  import
> org.ofbiz.base.util.UtilDateTime;
> >>>>  import
> org.ofbiz.base.util.UtilGenerics;
> >>>> @@ -316,44 +317,41 @@ public class
> WidgetWorker {
> >>>>          public
> String getValue(Map<String,
> Object>   context) {
> >>>>           
>   if (this.value != null) {
> >>>>           
>       return
> this.value.expandString(context);
> >>>> -           
> } else if (this.fromField !=
> null&&   this.fromField.get(context)
> != null) {
> >>>> -           
>     Object retVal = this.fromField.get(context);
> >>>> +           
> }
> >>>> +
> >>>> +           
> Object retVal = null;
> >>>> +           
> if (this.fromField !=
> null&&   this.fromField.get(context)
> != null) {
> >>>> +           
>     retVal = this.fromField.get(context);
> >>>> +           
> } else {
> >>>> +           
>     retVal = context.get(this.name);
> >>>> +           
> }
> >>>> +
> >>>> +           
> if (retVal != null) {
> >>>> +           
>     TimeZone timeZone = (TimeZone)
> context.get("timeZone");
> >>>> +           
>     if (timeZone == null) timeZone =
> TimeZone.getDefault();
> >>>>
> >>>> -           
>     if (retVal != null) {
> >>>> -           
>         TimeZone timeZone = (TimeZone)
> context.get("timeZone");
> >>>> -           
>         if (timeZone == null) timeZone =
> TimeZone.getDefault();
> >>>> -
> >>>> -           
>         String returnValue = null;
> >>>> -           
>         // format string based on the
> user's time zone (not locale because these are parameters)
> >>>> -           
>         if (retVal instanceof Double ||
> retVal instanceof Float || retVal instanceof BigDecimal) {
> >>>> -           
>             returnValue =
> retVal.toString();
> >>>> -           
>         } else if (retVal instanceof
> java.sql.Date) {
> >>>> -           
>             DateFormat df =
> UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT,
> timeZone, null);
> >>>> -           
>             returnValue =
> df.format((java.util.Date) retVal);
> >>>> -           
>         } else if (retVal instanceof
> java.sql.Time) {
> >>>> -           
>             DateFormat df =
> UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT,
> timeZone, null);
> >>>> -           
>             returnValue =
> df.format((java.util.Date) retVal);
> >>>> -           
>         } else if (retVal instanceof
> java.sql.Timestamp) {
> >>>> -           
>             DateFormat df =
> UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT,
> timeZone, null);
> >>>> -           
>             returnValue =
> df.format((java.util.Date) retVal);
> >>>> -           
>         } else if (retVal instanceof
> java.util.Date) {
> >>>> -           
>             DateFormat df =
> UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy",
> timeZone, null);
> >>>> -           
>             returnValue =
> df.format((java.util.Date) retVal);
> >>>> -           
>         } else {
> >>>> -           
>             returnValue =
> retVal.toString();
> >>>> -           
>         }
> >>>> -           
>         return returnValue;
> >>>> +           
>     String returnValue = null;
> >>>> +           
>     // format string based on the user's time zone
> (not locale because these are parameters)
> >>>> +           
>     if (retVal instanceof Double || retVal
> instanceof Float || retVal instanceof BigDecimal) {
> >>>> +           
>         returnValue =
> retVal.toString();
> >>>> +           
>     } else if (retVal instanceof java.sql.Date) {
> >>>> +           
>         DateFormat df =
> UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT,
> timeZone, null);
> >>>> +           
>         returnValue =
> df.format((java.util.Date) retVal);
> >>>> +           
>     } else if (retVal instanceof java.sql.Time) {
> >>>> +           
>         DateFormat df =
> UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT,
> timeZone, null);
> >>>> +           
>         returnValue =
> df.format((java.util.Date) retVal);
> >>>> +           
>     } else if (retVal instanceof
> java.sql.Timestamp) {
> >>>> +           
>         DateFormat df =
> UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT,
> timeZone, null);
> >>>> +           
>         returnValue =
> df.format((java.util.Date) retVal);
> >>>> +           
>     } else if (retVal instanceof java.util.Date)
> {
> >>>> +           
>         DateFormat df =
> UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy",
> timeZone, null);
> >>>> +           
>         returnValue =
> df.format((java.util.Date) retVal);
> >>>>           
>       } else {
> >>>> -           
>         return null;
> >>>> +           
>         returnValue =
> retVal.toString();
> >>>>           
>       }
> >>>> +           
>     return returnValue;
> >>>>           
>   } else {
> >>>> -           
>     // as a last chance try finding a context
> field with the key of the name field
> >>>> -           
>     Object obj = context.get(this.name);
> >>>> -           
>     if (obj != null) {
> >>>> -           
>         return obj.toString();
> >>>> -           
>     } else {
> >>>> -           
>         return null;
> >>>> -           
>     }
> >>>> +           
>     return null;
> >>>>           
>   }
> >>>>          }
> >>>>      }
> >>>>
> >>>>
> >>>>
> >>
> >>
>
>



Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r943843 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

David E. Jones-2

On May 13, 2010, at 8:56 PM, Adrian Crum wrote:

> Using the long value for date/time parameters is probably the most reliable way to do it. The Work Effort calendar does it that way for some URL parameters.
>
> Why use a custom type for parameters? You answered that question yourself:
>
> "It might actually be helpful to change how system generated object Strings are done, ie those not meant for human consumption..."
>
> Somewhere in the execution path there needs to be a way to flag data types as being intended to be used as parameters. My suggestion is to encapsulate the data in a Parameter data type, it sounds like your suggestion is to flag data types in some way or have data types destined for parameters be routed to some other execution path.

No, I didn't mean anything to do with data types. If something is a parameter in many cases in OFBiz the tool knows it and can do that automatically.

For other cases where parameters are assembled by code I asked why use a custom type because there are certainly alternatives to that, like calling a different method to encode it for system use instead of for human use.

I guess IMO use of custom types is not always the best way to give options, and actually to be honest IMO it's not a very good way (confusing, cumbersome, etc). Maybe I just like using java.* or other existing APIs instead of creating my own all the time (well, these days anyway... and I apologize for not doing this as much in the early days of OFBiz and thereby setting a precedent and direction for more of this).

-David


> I think it boils down to two different ways of looking at the problem.
>
> -Adrian
>
> --- On Thu, 5/13/10, David E Jones <[hidden email]> wrote:
>> Sorry, I meant for those questions to be an aspect of
>> rhetoric instead of trying to write out some sort of
>> requirement idea or issue.
>>
>> Whatever the case, your answer #3 is interesting... why use
>> a custom type with encoding/decoding when the current SQL
>> Date, Time, and Timestamp do so just fine (as long as you
>> use it's default toString and valueOf methods, with TimeZone
>> adjustment because the server will do that because we don't
>> distinguish between user input and system generated text).
>> It might actually be helpful to change how system generated
>> object Strings are done, ie those not meant for human
>> consumption, and for dates/times we could use an absolute
>> measure (like the millis from epoch) so that we don't have
>> to know which timezone the user is in to find the right
>> record in the database.
>>
>> I'm not sure if that's a good idea, but thought I'd throw
>> it out there...
>>
>> -David
>>
>>
>> On May 13, 2010, at 10:18 AM, Adrian Crum wrote:
>>
>>> 1. I don't know that it is official - it's more common
>> sense: If code exists to convert an object from one type to
>> another, why write more code that does the same thing?
>>>
>>> 2. I don't know. We would have to look for similar
>> code.
>>>
>>> 3. The best thing to do would be to use stronger data
>> typing. An HTTP (or URL) parameter should be a type -
>> instead of using a String. If we had a type for that, then
>> all kinds of convenient behavior could be attached to it
>> (like encoding/decoding, etc).
>>>
>>> -Adrian
>>>
>>> On 5/13/2010 9:09 AM, David E Jones wrote:
>>>>
>>>> Which brings up some questions:
>>>>
>>>> 1. is there an "official" way to convert types
>> that should be used everywhere (I think that's what you're
>> saying)?
>>>> 2. how many things are not using that?
>>>> 3. does the official way handle Object ->
>> String conversions for human versus machine (ie HTTP
>> parameter) use?
>>>>
>>>> -David
>>>>
>>>>
>>>> On May 13, 2010, at 8:54 AM, Adrian Crum wrote:
>>>>
>>>>> Just an observation, not commenting on this
>> commit specifically:
>>>>>
>>>>> The conversion to String should use the
>> ObjectType.simpleTypeConvert(...) method - there are a lot
>> of conversions being missed in this code.
>>>>>
>>>>> -Adrian
>>>>>
>>>>> On 5/13/2010 1:32 AM, [hidden email]
>> wrote:
>>>>>> Author: jonesde
>>>>>> Date: Thu May 13 08:32:09 2010
>>>>>> New Revision: 943843
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=943843&view=rev
>>>>>> Log:
>>>>>> Made widget link parameter treatment more
>> consistent; before if you specified a paramaeter name
>> without a from-field then it wouldn't convert the object
>> according to locale; now it does the same thing no matter
>> where the object comes from (ie explicit from-field or
>> implied by the parameter name); this fixes a timeZone
>> inconsistency that causes links with time/timestamp
>> parameters to not work
>>>>>>
>>>>>> Modified:
>>>>>>  
>>    ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>>>>>>
>>>>>> Modified:
>> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=943843&r1=943842&r2=943843&view=diff
>>>>>>
>> ==============================================================================
>>>>>> ---
>> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>> (original)
>>>>>> +++
>> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
>> Thu May 13 08:32:09 2010
>>>>>> @@ -30,6 +30,7 @@ import
>> javax.servlet.ServletContext;
>>>>>>   import
>> javax.servlet.http.HttpServletRequest;
>>>>>>   import
>> javax.servlet.http.HttpServletResponse;
>>>>>>
>>>>>> +import org.ofbiz.base.util.Debug;
>>>>>>   import
>> org.ofbiz.base.util.StringUtil;
>>>>>>   import
>> org.ofbiz.base.util.UtilDateTime;
>>>>>>   import
>> org.ofbiz.base.util.UtilGenerics;
>>>>>> @@ -316,44 +317,41 @@ public class
>> WidgetWorker {
>>>>>>           public
>> String getValue(Map<String,
>> Object>   context) {
>>>>>>            
>>   if (this.value != null) {
>>>>>>            
>>       return
>> this.value.expandString(context);
>>>>>> -          
>> } else if (this.fromField !=
>> null&&   this.fromField.get(context)
>> != null) {
>>>>>> -          
>>     Object retVal = this.fromField.get(context);
>>>>>> +          
>> }
>>>>>> +
>>>>>> +          
>> Object retVal = null;
>>>>>> +          
>> if (this.fromField !=
>> null&&   this.fromField.get(context)
>> != null) {
>>>>>> +          
>>     retVal = this.fromField.get(context);
>>>>>> +          
>> } else {
>>>>>> +          
>>     retVal = context.get(this.name);
>>>>>> +          
>> }
>>>>>> +
>>>>>> +          
>> if (retVal != null) {
>>>>>> +          
>>     TimeZone timeZone = (TimeZone)
>> context.get("timeZone");
>>>>>> +          
>>     if (timeZone == null) timeZone =
>> TimeZone.getDefault();
>>>>>>
>>>>>> -          
>>     if (retVal != null) {
>>>>>> -          
>>         TimeZone timeZone = (TimeZone)
>> context.get("timeZone");
>>>>>> -          
>>         if (timeZone == null) timeZone =
>> TimeZone.getDefault();
>>>>>> -
>>>>>> -          
>>         String returnValue = null;
>>>>>> -          
>>         // format string based on the
>> user's time zone (not locale because these are parameters)
>>>>>> -          
>>         if (retVal instanceof Double ||
>> retVal instanceof Float || retVal instanceof BigDecimal) {
>>>>>> -          
>>             returnValue =
>> retVal.toString();
>>>>>> -          
>>         } else if (retVal instanceof
>> java.sql.Date) {
>>>>>> -          
>>             DateFormat df =
>> UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT,
>> timeZone, null);
>>>>>> -          
>>             returnValue =
>> df.format((java.util.Date) retVal);
>>>>>> -          
>>         } else if (retVal instanceof
>> java.sql.Time) {
>>>>>> -          
>>             DateFormat df =
>> UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT,
>> timeZone, null);
>>>>>> -          
>>             returnValue =
>> df.format((java.util.Date) retVal);
>>>>>> -          
>>         } else if (retVal instanceof
>> java.sql.Timestamp) {
>>>>>> -          
>>             DateFormat df =
>> UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT,
>> timeZone, null);
>>>>>> -          
>>             returnValue =
>> df.format((java.util.Date) retVal);
>>>>>> -          
>>         } else if (retVal instanceof
>> java.util.Date) {
>>>>>> -          
>>             DateFormat df =
>> UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy",
>> timeZone, null);
>>>>>> -          
>>             returnValue =
>> df.format((java.util.Date) retVal);
>>>>>> -          
>>         } else {
>>>>>> -          
>>             returnValue =
>> retVal.toString();
>>>>>> -          
>>         }
>>>>>> -          
>>         return returnValue;
>>>>>> +          
>>     String returnValue = null;
>>>>>> +          
>>     // format string based on the user's time zone
>> (not locale because these are parameters)
>>>>>> +          
>>     if (retVal instanceof Double || retVal
>> instanceof Float || retVal instanceof BigDecimal) {
>>>>>> +          
>>         returnValue =
>> retVal.toString();
>>>>>> +          
>>     } else if (retVal instanceof java.sql.Date) {
>>>>>> +          
>>         DateFormat df =
>> UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT,
>> timeZone, null);
>>>>>> +          
>>         returnValue =
>> df.format((java.util.Date) retVal);
>>>>>> +          
>>     } else if (retVal instanceof java.sql.Time) {
>>>>>> +          
>>         DateFormat df =
>> UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT,
>> timeZone, null);
>>>>>> +          
>>         returnValue =
>> df.format((java.util.Date) retVal);
>>>>>> +          
>>     } else if (retVal instanceof
>> java.sql.Timestamp) {
>>>>>> +          
>>         DateFormat df =
>> UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT,
>> timeZone, null);
>>>>>> +          
>>         returnValue =
>> df.format((java.util.Date) retVal);
>>>>>> +          
>>     } else if (retVal instanceof java.util.Date)
>> {
>>>>>> +          
>>         DateFormat df =
>> UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss z yyyy",
>> timeZone, null);
>>>>>> +          
>>         returnValue =
>> df.format((java.util.Date) retVal);
>>>>>>            
>>       } else {
>>>>>> -          
>>         return null;
>>>>>> +          
>>         returnValue =
>> retVal.toString();
>>>>>>            
>>       }
>>>>>> +          
>>     return returnValue;
>>>>>>            
>>   } else {
>>>>>> -          
>>     // as a last chance try finding a context
>> field with the key of the name field
>>>>>> -          
>>     Object obj = context.get(this.name);
>>>>>> -          
>>     if (obj != null) {
>>>>>> -          
>>         return obj.toString();
>>>>>> -          
>>     } else {
>>>>>> -          
>>         return null;
>>>>>> -          
>>     }
>>>>>> +          
>>     return null;
>>>>>>            
>>   }
>>>>>>           }
>>>>>>       }
>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r943843 - /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java

Adrian Crum-2
--- On Thu, 5/13/10, David E Jones <[hidden email]> wrote:

> On May 13, 2010, at 8:56 PM, Adrian Crum wrote:
>
> > Using the long value for date/time parameters is
> probably the most reliable way to do it. The Work Effort
> calendar does it that way for some URL parameters.
> >
> > Why use a custom type for parameters? You answered
> that question yourself:
> >
> > "It might actually be helpful to change how system
> generated object Strings are done, ie those not meant for
> human consumption..."
> >
> > Somewhere in the execution path there needs to be a
> way to flag data types as being intended to be used as
> parameters. My suggestion is to encapsulate the data in a
> Parameter data type, it sounds like your suggestion is to
> flag data types in some way or have data types destined for
> parameters be routed to some other execution path.
>
> No, I didn't mean anything to do with data types. If
> something is a parameter in many cases in OFBiz the tool
> knows it and can do that automatically.
>
> For other cases where parameters are assembled by code I
> asked why use a custom type because there are certainly
> alternatives to that, like calling a different method to
> encode it for system use instead of for human use.
>
> I guess IMO use of custom types is not always the best way
> to give options, and actually to be honest IMO it's not a
> very good way (confusing, cumbersome, etc). Maybe I just
> like using java.* or other existing APIs instead of creating
> my own all the time (well, these days anyway... and I
> apologize for not doing this as much in the early days of
> OFBiz and thereby setting a precedent and direction for more
> of this).

I understand. What you're saying is, if you have a Map in your hand and you know the contents of that Map need to converted to a URL query string, you pass the Map to a utility method that converts it to a URL query string. The approach I'm suggesting is you have a URLParameters object in your hand, and you call the object's toString() method (or toQueryString() method) to get the URL query string.

Conversely, to convert a URL query string to a Map, what you're saying is you would pass the query string to a utility method to perform the conversion, and the approach I'm suggesting is you pass the query string to the URLParameters.fromQueryString factory method.

With your approach the desired behavior is external and with my approach it's internal.

-Adrian


> > I think it boils down to two different ways of looking
> at the problem.
> >
> > -Adrian
> >
> > --- On Thu, 5/13/10, David E Jones <[hidden email]>
> wrote:
> >> Sorry, I meant for those questions to be an aspect
> of
> >> rhetoric instead of trying to write out some sort
> of
> >> requirement idea or issue.
> >>
> >> Whatever the case, your answer #3 is
> interesting... why use
> >> a custom type with encoding/decoding when the
> current SQL
> >> Date, Time, and Timestamp do so just fine (as long
> as you
> >> use it's default toString and valueOf methods,
> with TimeZone
> >> adjustment because the server will do that because
> we don't
> >> distinguish between user input and system
> generated text).
> >> It might actually be helpful to change how system
> generated
> >> object Strings are done, ie those not meant for
> human
> >> consumption, and for dates/times we could use an
> absolute
> >> measure (like the millis from epoch) so that we
> don't have
> >> to know which timezone the user is in to find the
> right
> >> record in the database.
> >>
> >> I'm not sure if that's a good idea, but thought
> I'd throw
> >> it out there...
> >>
> >> -David
> >>
> >>
> >> On May 13, 2010, at 10:18 AM, Adrian Crum wrote:
> >>
> >>> 1. I don't know that it is official - it's
> more common
> >> sense: If code exists to convert an object from
> one type to
> >> another, why write more code that does the same
> thing?
> >>>
> >>> 2. I don't know. We would have to look for
> similar
> >> code.
> >>>
> >>> 3. The best thing to do would be to use
> stronger data
> >> typing. An HTTP (or URL) parameter should be a
> type -
> >> instead of using a String. If we had a type for
> that, then
> >> all kinds of convenient behavior could be attached
> to it
> >> (like encoding/decoding, etc).
> >>>
> >>> -Adrian
> >>>
> >>> On 5/13/2010 9:09 AM, David E Jones wrote:
> >>>>
> >>>> Which brings up some questions:
> >>>>
> >>>> 1. is there an "official" way to convert
> types
> >> that should be used everywhere (I think that's
> what you're
> >> saying)?
> >>>> 2. how many things are not using that?
> >>>> 3. does the official way handle Object
> ->
> >> String conversions for human versus machine (ie
> HTTP
> >> parameter) use?
> >>>>
> >>>> -David
> >>>>
> >>>>
> >>>> On May 13, 2010, at 8:54 AM, Adrian Crum
> wrote:
> >>>>
> >>>>> Just an observation, not commenting on
> this
> >> commit specifically:
> >>>>>
> >>>>> The conversion to String should use
> the
> >> ObjectType.simpleTypeConvert(...) method - there
> are a lot
> >> of conversions being missed in this code.
> >>>>>
> >>>>> -Adrian
> >>>>>
> >>>>> On 5/13/2010 1:32 AM, [hidden email]
> >> wrote:
> >>>>>> Author: jonesde
> >>>>>> Date: Thu May 13 08:32:09 2010
> >>>>>> New Revision: 943843
> >>>>>>
> >>>>>> URL: http://svn.apache.org/viewvc?rev=943843&view=rev
> >>>>>> Log:
> >>>>>> Made widget link parameter
> treatment more
> >> consistent; before if you specified a paramaeter
> name
> >> without a from-field then it wouldn't convert the
> object
> >> according to locale; now it does the same thing no
> matter
> >> where the object comes from (ie explicit
> from-field or
> >> implied by the parameter name); this fixes a
> timeZone
> >> inconsistency that causes links with
> time/timestamp
> >> parameters to not work
> >>>>>>
> >>>>>> Modified:
> >>>>>> 
> >>   
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> >>>>>>
> >>>>>> Modified:
> >>
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> >>>>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=943843&r1=943842&r2=943843&view=diff
> >>>>>>
> >>
> ==============================================================================
> >>>>>> ---
> >>
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> >> (original)
> >>>>>> +++
> >>
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java
> >> Thu May 13 08:32:09 2010
> >>>>>> @@ -30,6 +30,7 @@ import
> >> javax.servlet.ServletContext;
> >>>>>>   import
> >> javax.servlet.http.HttpServletRequest;
> >>>>>>   import
> >> javax.servlet.http.HttpServletResponse;
> >>>>>>
> >>>>>> +import
> org.ofbiz.base.util.Debug;
> >>>>>>   import
> >> org.ofbiz.base.util.StringUtil;
> >>>>>>   import
> >> org.ofbiz.base.util.UtilDateTime;
> >>>>>>   import
> >> org.ofbiz.base.util.UtilGenerics;
> >>>>>> @@ -316,44 +317,41 @@ public
> class
> >> WidgetWorker {
> >>>>>>       
>    public
> >> String getValue(Map<String,
> >> Object>   context) {
> >>>>>>         
>  
> >>   if (this.value != null) {
> >>>>>>         
>  
> >>       return
> >> this.value.expandString(context);
> >>>>>> -       
>    
> >> } else if (this.fromField !=
> >>
> null&&   this.fromField.get(context)
> >> != null) {
> >>>>>> -       
>    
> >>     Object retVal =
> this.fromField.get(context);
> >>>>>> +       
>    
> >> }
> >>>>>> +
> >>>>>> +       
>    
> >> Object retVal = null;
> >>>>>> +       
>    
> >> if (this.fromField !=
> >>
> null&&   this.fromField.get(context)
> >> != null) {
> >>>>>> +       
>    
> >>     retVal =
> this.fromField.get(context);
> >>>>>> +       
>    
> >> } else {
> >>>>>> +       
>    
> >>     retVal =
> context.get(this.name);
> >>>>>> +       
>    
> >> }
> >>>>>> +
> >>>>>> +       
>    
> >> if (retVal != null) {
> >>>>>> +       
>    
> >>     TimeZone timeZone =
> (TimeZone)
> >> context.get("timeZone");
> >>>>>> +       
>    
> >>     if (timeZone == null)
> timeZone =
> >> TimeZone.getDefault();
> >>>>>>
> >>>>>> -       
>    
> >>     if (retVal != null) {
> >>>>>> -       
>    
> >>         TimeZone
> timeZone = (TimeZone)
> >> context.get("timeZone");
> >>>>>> -       
>    
> >>         if (timeZone
> == null) timeZone =
> >> TimeZone.getDefault();
> >>>>>> -
> >>>>>> -       
>    
> >>         String
> returnValue = null;
> >>>>>> -       
>    
> >>         // format
> string based on the
> >> user's time zone (not locale because these are
> parameters)
> >>>>>> -       
>    
> >>         if (retVal
> instanceof Double ||
> >> retVal instanceof Float || retVal instanceof
> BigDecimal) {
> >>>>>> -       
>    
> >>         
>    returnValue =
> >> retVal.toString();
> >>>>>> -       
>    
> >>         } else if
> (retVal instanceof
> >> java.sql.Date) {
> >>>>>> -       
>    
> >>         
>    DateFormat df =
> >>
> UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT,
> >> timeZone, null);
> >>>>>> -       
>    
> >>         
>    returnValue =
> >> df.format((java.util.Date) retVal);
> >>>>>> -       
>    
> >>         } else if
> (retVal instanceof
> >> java.sql.Time) {
> >>>>>> -       
>    
> >>         
>    DateFormat df =
> >>
> UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT,
> >> timeZone, null);
> >>>>>> -       
>    
> >>         
>    returnValue =
> >> df.format((java.util.Date) retVal);
> >>>>>> -       
>    
> >>         } else if
> (retVal instanceof
> >> java.sql.Timestamp) {
> >>>>>> -       
>    
> >>         
>    DateFormat df =
> >>
> UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT,
> >> timeZone, null);
> >>>>>> -       
>    
> >>         
>    returnValue =
> >> df.format((java.util.Date) retVal);
> >>>>>> -       
>    
> >>         } else if
> (retVal instanceof
> >> java.util.Date) {
> >>>>>> -       
>    
> >>         
>    DateFormat df =
> >> UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss
> z yyyy",
> >> timeZone, null);
> >>>>>> -       
>    
> >>         
>    returnValue =
> >> df.format((java.util.Date) retVal);
> >>>>>> -       
>    
> >>         } else {
> >>>>>> -       
>    
> >>         
>    returnValue =
> >> retVal.toString();
> >>>>>> -       
>    
> >>         }
> >>>>>> -       
>    
> >>         return
> returnValue;
> >>>>>> +       
>    
> >>     String returnValue =
> null;
> >>>>>> +       
>    
> >>     // format string based on
> the user's time zone
> >> (not locale because these are parameters)
> >>>>>> +       
>    
> >>     if (retVal instanceof
> Double || retVal
> >> instanceof Float || retVal instanceof BigDecimal)
> {
> >>>>>> +       
>    
> >>         returnValue
> =
> >> retVal.toString();
> >>>>>> +       
>    
> >>     } else if (retVal
> instanceof java.sql.Date) {
> >>>>>> +       
>    
> >>         DateFormat
> df =
> >>
> UtilDateTime.toDateFormat(UtilDateTime.DATE_FORMAT,
> >> timeZone, null);
> >>>>>> +       
>    
> >>         returnValue
> =
> >> df.format((java.util.Date) retVal);
> >>>>>> +       
>    
> >>     } else if (retVal
> instanceof java.sql.Time) {
> >>>>>> +       
>    
> >>         DateFormat
> df =
> >>
> UtilDateTime.toTimeFormat(UtilDateTime.TIME_FORMAT,
> >> timeZone, null);
> >>>>>> +       
>    
> >>         returnValue
> =
> >> df.format((java.util.Date) retVal);
> >>>>>> +       
>    
> >>     } else if (retVal
> instanceof
> >> java.sql.Timestamp) {
> >>>>>> +       
>    
> >>         DateFormat
> df =
> >>
> UtilDateTime.toDateTimeFormat(UtilDateTime.DATE_TIME_FORMAT,
> >> timeZone, null);
> >>>>>> +       
>    
> >>         returnValue
> =
> >> df.format((java.util.Date) retVal);
> >>>>>> +       
>    
> >>     } else if (retVal
> instanceof java.util.Date)
> >> {
> >>>>>> +       
>    
> >>         DateFormat
> df =
> >> UtilDateTime.toDateTimeFormat("EEE MMM dd hh:mm:ss
> z yyyy",
> >> timeZone, null);
> >>>>>> +       
>    
> >>         returnValue
> =
> >> df.format((java.util.Date) retVal);
> >>>>>>         
>  
> >>       } else {
> >>>>>> -       
>    
> >>         return
> null;
> >>>>>> +       
>    
> >>         returnValue
> =
> >> retVal.toString();
> >>>>>>         
>  
> >>       }
> >>>>>> +       
>    
> >>     return returnValue;
> >>>>>>         
>  
> >>   } else {
> >>>>>> -       
>    
> >>     // as a last chance try
> finding a context
> >> field with the key of the name field
> >>>>>> -       
>    
> >>     Object obj =
> context.get(this.name);
> >>>>>> -       
>    
> >>     if (obj != null) {
> >>>>>> -       
>    
> >>         return
> obj.toString();
> >>>>>> -       
>    
> >>     } else {
> >>>>>> -       
>    
> >>         return
> null;
> >>>>>> -       
>    
> >>     }
> >>>>>> +       
>    
> >>     return null;
> >>>>>>         
>  
> >>   }
> >>>>>>       
>    }
> >>>>>>       }
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>
> >>>>
> >>
> >>
> >
> >
> >
>
>