Skip to content

Commit a23583a

Browse files
committed
Fix many conversion-related issues
* Refactor DefaultConverter, fixing many bugs, and updating unit tests accordingly. * Uncomment tests in DefaultConverterTest, which now pass. * Undeprecate canConvert methods with Class src. There is no replacement when an object of the source type is not available. * Make ConversionUtils better match ConvertService. It now uses the list of built-in converters that are part of SciJava Common, in the same order they are used as Converter plugins. * Clean up conversion-related classes: * Add missing final keywords, fix warnings, and clean up style. * Update javadoc comments to be more accurate in more places. * Fix NullConverter to take precedence iff src and/or dest is null.
1 parent f6856fd commit a23583a

17 files changed

+310
-312
lines changed

src/main/java/org/scijava/convert/ArrayToStringConverter.java

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,11 @@
3737
import org.scijava.plugin.Parameter;
3838
import org.scijava.plugin.Plugin;
3939
import org.scijava.util.ArrayUtils;
40-
import org.scijava.util.Types;
4140

4241
/**
43-
* A {@link Converter} that specializes in converting
44-
* n-dimensional arrays into {@link String}s. This {@link Converter} can convert any array whose
45-
* component types can be converted into {@link String}s. By default, this
42+
* A {@link Converter} that specializes in converting n-dimensional arrays into
43+
* {@link String}s. This {@link Converter} can convert any array whose component
44+
* types can be converted into {@link String}s. By default, this
4645
* {@link Converter} delimits the array elements with commas.
4746
*
4847
* @author Gabriel Selzer
@@ -53,77 +52,77 @@ public class ArrayToStringConverter extends AbstractConverter<Object, String> {
5352
@Parameter(required = false)
5453
private ConvertService convertService;
5554

56-
@Override public boolean canConvert(final Class<?> src, final Class<?> dest) {
57-
if (src == null) return false;
58-
final Class<?> saneSrc = Types.box(src);
59-
final Class<?> saneDest = Types.box(dest);
60-
return saneSrc.isArray() && saneDest == String.class;
55+
@Override
56+
public boolean canConvert(final Class<?> src, final Class<?> dest) {
57+
return src != null && src.isArray() && dest == String.class;
6158
}
6259

63-
@Override public boolean canConvert(final Object src, final Class<?> dest) {
64-
if (convertService == null) return false;
60+
@Override
61+
public boolean canConvert(final Object src, final Class<?> dest) {
62+
if (convertService == null || src == null) return false;
6563
if (!canConvert(src.getClass(), dest)) return false;
6664
if (Array.getLength(src) == 0) return true;
6765
return convertService.supports(Array.get(src, 0), dest);
6866
}
6967

7068
@Override
71-
public Object convert(Object src, Type dest) {
69+
public Object convert(Object src, final Type dest) {
7270
// Preprocess the "string-likes"
73-
if (src.getClass().equals(String[].class) || //
74-
src.getClass().equals(Character[].class) || //
75-
src.getClass().equals(char[].class)) //
71+
final Class<?> srcClass = src.getClass();
72+
if (srcClass == String[].class || //
73+
srcClass == Character[].class || //
74+
srcClass == char[].class) //
7675
{
7776
src = preprocessCharacters(src);
7877
}
7978
// Convert each element to Strings
80-
String elementString = ArrayUtils.toCollection(src).stream() //
79+
final String elementString = ArrayUtils.toCollection(src).stream() //
8180
.map(object -> convertService.convert(object, String.class)) //
8281
.collect(Collectors.joining(", "));
8382
return "{" + elementString + "}";
8483
}
8584

8685
private String[] preprocessStrings(final Object src) {
87-
int numElements = Array.getLength(src);
88-
String[] processed = new String[numElements];
86+
final int numElements = Array.getLength(src);
87+
final String[] processed = new String[numElements];
8988
for (int i = 0; i < numElements; i++) {
9089
processed[i] = preprocessString(Array.get(src, i));
9190
}
9291
return processed;
9392
}
9493

9594
private String preprocessString(final Object o) {
96-
String s;
97-
if (o == null) {
98-
return null;
99-
} else {
100-
s = o.toString();
101-
s = s.replace("\\", "\\\\");
102-
s = s.replace("\"", "\\\"");
103-
}
95+
if (o == null) return null;
96+
String s = o.toString();
97+
s = s.replace("\\", "\\\\");
98+
s = s.replace("\"", "\\\"");
10499
return "\"" + s + "\"";
105100
}
106101

107102
private String[] preprocessCharacters(Object src) {
108-
String[] processed = new String[Array.getLength(src)];
103+
final String[] processed = new String[Array.getLength(src)];
109104
for (int i = 0; i < processed.length; i++) {
110-
Object value = Array.get(src, i);
105+
final Object value = Array.get(src, i);
111106
processed[i] = value == null ? null : value.toString();
112107
}
113108
return preprocessStrings(processed);
114109
}
115110

116-
@SuppressWarnings("unchecked") @Override
117-
public <T> T convert(Object src, Class<T> dest) {
118-
return (T) convert(src, (Type) dest);
111+
@Override
112+
public <T> T convert(final Object src, final Class<T> dest) {
113+
final Type destType = dest;
114+
@SuppressWarnings("unchecked")
115+
final T converted = (T) convert(src, destType);
116+
return converted;
119117
}
120118

121-
@Override public Class<String> getOutputType() {
119+
@Override
120+
public Class<String> getOutputType() {
122121
return String.class;
123122
}
124123

125-
@Override public Class<Object> getInputType() {
124+
@Override
125+
public Class<Object> getInputType() {
126126
return Object.class;
127127
}
128-
129128
}

src/main/java/org/scijava/convert/CastingConverter.java

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
*/
2929
package org.scijava.convert;
3030

31+
import java.lang.reflect.Type;
32+
3133
import org.scijava.Priority;
3234
import org.scijava.plugin.Plugin;
3335
import org.scijava.util.Types;
@@ -37,7 +39,7 @@
3739
*
3840
* @author Mark Hiner
3941
*/
40-
@Plugin(type = Converter.class, priority = Priority.EXTREMELY_HIGH)
42+
@Plugin(type = Converter.class, priority = Priority.EXTREMELY_HIGH - 1)
4143
public class CastingConverter extends AbstractConverter<Object, Object> {
4244

4345
@Override
@@ -46,25 +48,23 @@ public boolean canConvert(final Object src, final Class<?> dest) {
4648
}
4749

4850
@Override
49-
public boolean canConvert(final Class<?> src, final Class<?> dest) {
51+
public boolean canConvert(final Class<?> src, final Type dest) {
5052
// OK if the existing object can be casted
5153
return dest != null && Types.isAssignable(src, dest);
5254
}
5355

54-
@SuppressWarnings("unchecked")
56+
@Override
57+
public boolean canConvert(final Class<?> src, final Class<?> dest) {
58+
// NB: Invert functional flow from Converter interface:
59+
// Converter: canConvert(Class, Type) -> canConvert(Class, Class)
60+
// becomes: canConvert(Class, Class) -> canConvert(Class, Type)
61+
final Type destType = dest;
62+
return canConvert(src, destType);
63+
}
64+
5565
@Override
5666
public <T> T convert(final Object src, final Class<T> dest) {
57-
// NB: Regardless of whether the destination type is an array or
58-
// collection, we still want to cast directly if doing so is possible.
59-
// But note that in general, this check does not detect cases of
60-
// incompatible generic parameter types. If this limitation becomes a
61-
// problem in the future we can extend the logic here to provide
62-
// additional signatures of canCast which operate on Types in general
63-
// rather than only Classes. However, the logic could become complex
64-
// very quickly in various subclassing cases, generic parameters
65-
// resolved vs. propagated, etc.
66-
final Class<?> c = Types.raw(dest);
67-
return (T) Types.cast(src, c);
67+
return Types.cast(src, dest);
6868
}
6969

7070
@Override

src/main/java/org/scijava/convert/DefaultConvertService.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,5 @@
4040
@Plugin(type = Service.class)
4141
public class DefaultConvertService extends AbstractConvertService
4242
{
43-
// Trivial implementation
43+
// NB: No implementation needed.
4444
}

0 commit comments

Comments
 (0)