spring-projects
A new interface and implementation for FieldSet [BATCH-1560]
Hussachai opened BATCH-1560 and commented
An existing design of org.springframework.batch.item.file.transform.FieldSet is not comfortable to program with (at least for me)
The first disadavantage.
Because it uses primitive type. Converting value to primitive type may lead to NPE easily. I know this issue may be solved with defaultValue. But if I'd like to check null there are two ways to do this 1) define some defaultValue that represent as null 2) check the exception. I think many people don't think it's good idea to do that. They may recommend to check null || empty on readString() before further processing.
For example. cust.salary = fs.readInt("salary"); // if salary has no value NPE will be raised.
To solve above problem
cust.salary = fs.readInt("salary",-1); //application must know -1 mean null
if(StringUtils.isNotEmpty(fs.readString())){
cust.salary = fs.readInt("salary");
}
The second is more sense but it has more code to write too.
The proposed method is
cust.salary = fs.read(Integer.class, "salary", null); // I think this is the best way.
The second disadvantage
is the limit of conversion interface it just provides primitive type, date and number conversion. If I want more such as I want Email object, Money object, Class object, URL object how to do that? The current interface I must get the string and do the conversion manually.
See the attachment.
I don't know my idea is good enough or not. I just want to share the idea. For me now I will use this to be the wrapper for the DefaultFieldSet object.
Affects: 2.1.0
Attachments:
- transform.zip (1.46 kB)
Dave Syer commented
I'm not sure we would want to change the FieldSet interface so radically (removing all the primitive accessors), but adding new methods is probably something that could happen in 3.0.
Note that the type conversion to non primitive types can be done by a FieldSetMapper as well, and this is probably a more common and useful extension point (as you already are doing I imagine). Actually the BeanWrapperFieldSetMapper already does automatic type conversion. You might not want to use that for everything, but the idea is easy to copy: just use FieldSet.readString(..) and convert in the mapper.
An alternative or complenentary interim measure would be (as you suggest) to write a utility to wrap a field set that can be used by a mapper.
FieldSet was designed in a similar fashion to java.sql.ResultSet, which uses primitive types as well. I don't think using primitive types is an issue as long as the API allows you to check for the presence of a key. So checking for the presence of a key before accessing it is the way to go, if the "get-or-default" way of things is not enough.
Moreover, I agree with https://github.com/spring-projects/spring-batch/issues/2025#issuecomment-566289545, the FieldSetMapper is also a good candidate to convert data type, but I would keep that option as a last solution.
That said and while I believe we do not need a new interface as requested here, I am open to adding new methods for other data types if needed (I think of Java Date/Time types for example). But that would be another feature, so I am closing this one.
