Am 28.12.2014 um 23:53 schrieb sebb:
On 28 December 2014 at 20:34, Philippe Mouawad
<[email protected]> wrote:
On Sun, Dec 28, 2014 at 4:33 PM, Felix Schumacher <
[email protected]> wrote:
Hi all,
I have two questions about getCalendar(Object, Calendar) in Converter.
1)
Documentation says the default value would be the current date, while the
code shows it will be defaultValue (second parameter). I will correct the
documentation. (Well not really a question then :)
OK
+1
Done. Together with the other documentation commits we are now down to
only 460 errors due to missing javadoc param, return or throws attributes :)
2)
The code is quite deep because of the exception handling. I think it
would be nicer to use a for loop to iterate the possible formats. Should I
do so? (Now that is a question)
Yes, but as you say in 3) code seems a bit weird
-1
I would leave well alone.
3)
While the first (really the second) conversion uses date.toString() to
convert date to a String, the following conversions assume that date is a
String (using (String) date casts). That would be handled by a correct
implementation for 2), or could be handled separately. Should I do so? (If
so, by using for loop for 2, or just a simple toString() call?)
Code seems buggy or at least not clear for me, looking at callers it seems
parameter can be date or string. So if it's not date, it is string and I
don't understand in this case the toString
-1
I don't think it's a good idea to tamper with this method.
There are no unit tests for it, and it is used for preparing the TestBeans.
If any subtle changes are introduced it may prove very difficult to debug.
Very well, the unit tests are there already :) But as you are against
it, I will not submit the changes. However if anyone likes to look at
them, I have attached them.
Regards
Felix
Regards
Felix
--
Cordialement.
Philippe Mouawad.
>From a8bbdffeae307cc3842ff17d5b28c384ae128090 Mon Sep 17 00:00:00 2001
From: Felix Schumacher <[email protected]>
Date: Sun, 28 Dec 2014 18:22:19 +0100
Subject: [PATCH 1/2] Tests for getCalendar(Object, Calendar) and
getDate(Object, Date) in Converter
---
src/jorphan/org/apache/jorphan/util/Converter.java | 73 ++++++++--------------
1 file changed, 25 insertions(+), 48 deletions(-)
diff --git a/src/jorphan/org/apache/jorphan/util/Converter.java b/src/jorphan/org/apache/jorphan/util/Converter.java
index 98ec5f0..755923b 100644
--- a/src/jorphan/org/apache/jorphan/util/Converter.java
+++ b/src/jorphan/org/apache/jorphan/util/Converter.java
@@ -21,6 +21,7 @@ import java.io.File;
import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
+import java.util.Arrays;
import java.util.Calendar;
import java.util.Date;
import java.util.GregorianCalendar;
@@ -93,34 +94,23 @@ public class Converter {
if (date instanceof java.util.Date) {
cal.setTime((java.util.Date) date);
return cal;
- } else if (date != null) {
- DateFormat formatter = DateFormat.getDateInstance(DateFormat.SHORT);
- java.util.Date d = null;
- try {
- d = formatter.parse(date.toString());
- } catch (ParseException e) {
- formatter = DateFormat.getDateInstance(DateFormat.MEDIUM);
+ }
+ if (date != null) {
+ String dateAsString = date.toString();
+ for (int dateFormat : Arrays.asList(DateFormat.SHORT,
+ DateFormat.MEDIUM, DateFormat.LONG, DateFormat.FULL)) {
+ DateFormat formatter = DateFormat.getDateInstance(dateFormat);
try {
- d = formatter.parse((String) date);
- } catch (ParseException e1) {
- formatter = DateFormat.getDateInstance(DateFormat.LONG);
- try {
- d = formatter.parse((String) date);
- } catch (ParseException e2) {
- formatter = DateFormat.getDateInstance(DateFormat.FULL);
- try {
- d = formatter.parse((String) date);
- } catch (ParseException e3) {
- return defaultValue;
- }
- }
+ Date parsedDate = formatter.parse(dateAsString);
+ cal.setTime(parsedDate);
+ return cal;
+ } catch (ParseException e) {
+ // nothing to do here, just try the next date format, or
+ // fall through to return defaultValue
}
}
- cal.setTime(d);
- } else {
- cal = defaultValue;
}
- return cal;
+ return defaultValue;
}
/**
@@ -163,36 +153,23 @@ public class Converter {
* <code>defaultValue</code> if conversion failed
*/
public static Date getDate(Object date, Date defaultValue) {
- Date val = null;
if (date instanceof java.util.Date) {
return (Date) date;
- } else if (date != null) {
- DateFormat formatter = DateFormat.getDateInstance(DateFormat.SHORT);
- // java.util.Date d = null;
- try {
- val = formatter.parse(date.toString());
- } catch (ParseException e) {
- formatter = DateFormat.getDateInstance(DateFormat.MEDIUM);
+ }
+ if (date != null) {
+ String dateAsString = date.toString();
+ for (int dateFormat : Arrays.asList(DateFormat.SHORT,
+ DateFormat.MEDIUM, DateFormat.LONG, DateFormat.FULL)) {
+ DateFormat formatter = DateFormat.getDateInstance(dateFormat);
try {
- val = formatter.parse((String) date);
- } catch (ParseException e1) {
- formatter = DateFormat.getDateInstance(DateFormat.LONG);
- try {
- val = formatter.parse((String) date);
- } catch (ParseException e2) {
- formatter = DateFormat.getDateInstance(DateFormat.FULL);
- try {
- val = formatter.parse((String) date);
- } catch (ParseException e3) {
- return defaultValue;
- }
- }
+ return formatter.parse(dateAsString);
+ } catch (ParseException e) {
+ // nothing to do here, just try the next date format, or
+ // fall through to return defaultValue
}
}
- } else {
- return defaultValue;
}
- return val;
+ return defaultValue;
}
/**
--
1.9.1
