You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by "Dimitri Unruh (Created) (JIRA)" <ji...@apache.org> on 2011/11/07 16:32:51 UTC
[jira] [Created] (OFBIZ-4545) DateFormat API extension
DateFormat API extension
------------------------
Key: OFBIZ-4545
URL: https://issues.apache.org/jira/browse/OFBIZ-4545
Project: OFBiz
Issue Type: Improvement
Components: framework
Reporter: Dimitri Unruh
Priority: Minor
Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Adrian Crum (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13146384#comment-13146384 ]
Adrian Crum commented on OFBIZ-4545:
------------------------------------
That is precisely what I have been referring to. Freemarker = client code. The client code should pass valid arguments to the method. If there is a chance myGenericValue.date can be null, then the template needs to resolve it before calling the method.
The problem with low-level code accepting invalid arguments and returning arbitrary values is that there is no way of knowing if you've done something wrong when calling the method. Worse yet, everyone has a different opinion on what the default returned value should be - so you end up with a complicated and convoluted API.
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dimitri Unruh updated OFBIZ-4545:
---------------------------------
Attachment: OFBIZ-4545.patch
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13147674#comment-13147674 ]
Dimitri Unruh commented on OFBIZ-4545:
--------------------------------------
Thanks Adrian!
The existing method was the reason for my first try with an empty string :)
Anyway +1 for your explanations and also the review work!
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Assignee: Adrian Crum
> Priority: Minor
> Fix For: SVN trunk
>
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Adrian Crum (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13146255#comment-13146255 ]
Adrian Crum commented on OFBIZ-4545:
------------------------------------
The patch is looking better, but you are still checking one of the arguments for null, but this time you are returning null instead of an empty String. You need to remove the argument checks. Allow the method to throw NPE instead of returning arbitrary default values.
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Assigned] (OFBIZ-4545) DateFormat API extension
Posted by "Adrian Crum (Assigned) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adrian Crum reassigned OFBIZ-4545:
----------------------------------
Assignee: Adrian Crum
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Assignee: Adrian Crum
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13146241#comment-13146241 ]
Dimitri Unruh commented on OFBIZ-4545:
--------------------------------------
Again, thanks for the review Adrian :)
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Closed] (OFBIZ-4545) DateFormat API extension
Posted by "Adrian Crum (Closed) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Adrian Crum closed OFBIZ-4545.
------------------------------
Resolution: Fixed
Fix Version/s: SVN trunk
Fixed, rev 1199793. Thanks Dimitri!
I understand the existing methods in that class return an empty string if any of the parameters are null, but like I said earlier, I believe that is a bad design pattern. If another committer feels otherwise, they are welcome to change the methods to return an empty string if any of the parameters are null.
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Assignee: Adrian Crum
> Priority: Minor
> Fix For: SVN trunk
>
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dimitri Unruh updated OFBIZ-4545:
---------------------------------
Attachment: OFBIZ-4545.patch
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Adrian Crum (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13145591#comment-13145591 ]
Adrian Crum commented on OFBIZ-4545:
------------------------------------
This patch needs some work:
1. Do not add date/time format methods that do not include Locale and TimeZone parameters. All conversions from/to Date/String must include those objects for proper localization.
2. Do not provide defaults if the Locale or TimeZone parameters are null. Client code should provide the defaults - since UtilDateTime can't "know" what default values to provide.
3. There is no need to convert a Timestamp to a Date - since Timestamp extends Date. It would be best if the new methods accepted a Date argument instead of a Timestamp.
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dimitri Unruh updated OFBIZ-4545:
---------------------------------
Attachment: OFBIZ-4545.patch
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Adrian Crum (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13146167#comment-13146167 ]
Adrian Crum commented on OFBIZ-4545:
------------------------------------
The method:
public static String formatDate(Date date, DateFormat dateFormatter)
is not needed. If you already have both objects, then you can format the date with them.
Do not test for null arguments and return an empty String - that is a bad design pattern that makes development and debugging difficult. If client code passes a null argument to the method, then the method should throw a NPE - that is how you let the client code know that it is doing something wrong. See the current version of UtilDateTime.java, line 760 and below for the correct way to implement date/time methods.
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Dimitri Unruh updated OFBIZ-4545:
---------------------------------
Attachment: OFBIZ-4545.patch
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13146307#comment-13146307 ]
Dimitri Unruh commented on OFBIZ-4545:
--------------------------------------
I think it make sense to check the date argument for null ant return null at this case.
Think about the usage for example in freemarker templates:
{code}
${Static["org.ofbiz.base.util.UtilFormatOut"].formatDate(myGenericValue.date, ...)!}
{code}
If date in myGenericValue is empty I don't want to get a NPE :)
What do you think?
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13146147#comment-13146147 ]
Dimitri Unruh commented on OFBIZ-4545:
--------------------------------------
Adrian,
thanks for the review.
# I would prefer to deprecete the old method _*public static String formatDate(java.sql.Timestamp timestamp){...}*_ What do you think?
# okay
# My fault, I missed this point :)
I will create a new one...
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (OFBIZ-4545) DateFormat API extension
Posted by "Dimitri Unruh (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/OFBIZ-4545?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13146884#comment-13146884 ]
Dimitri Unruh commented on OFBIZ-4545:
--------------------------------------
Adrian,
thanks for the clarification. I will modify the patch :)
> DateFormat API extension
> ------------------------
>
> Key: OFBIZ-4545
> URL: https://issues.apache.org/jira/browse/OFBIZ-4545
> Project: OFBiz
> Issue Type: Improvement
> Components: framework
> Reporter: Dimitri Unruh
> Priority: Minor
> Attachments: OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch, OFBIZ-4545.patch
>
>
> Some API extensions for Dateformating
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira