Extending functionality of org.springframework.batch.item.file.transform.DefaultFieldSet

Question

I would like to be able to set token values (defaultFieldSet.tokens) and names (defaultFieldSet.names) on org.springframework.batch.item.file.transform.DefaultFieldSet using a java.util.Properties object. Specifically, the keys of the Properties object will serve as the names and the corresponding Properties values will serve as the tokens. Here's the code that I have to do this:

import java.util.Properties;
import java.util.Set;
import org.springframework.batch.item.file.transform.DefaultFieldSet;
import org.springframework.batch.item.file.transform.FieldSet;
/**
 * <code>PropertiesFieldSetFactory</code> is a factory to create
 * a {@link FieldSet} from a {@link Properties} object.
*/
public class PropertiesFieldSetFactory {
 /**
 * Creates a {@link FieldSet} by setting its token values equal to the {@link Properties} values
 * and its token names equal to the {@link Properties} keys. 
 * Note: Passing a null argument to this method will cause a {@link NullPointerException} 
 * to be thrown.
 * 
 * @param properties used to populate the {@link FieldSet}
 * @return {@link FieldSet} that has token values and names from the passed in {@link Properties} object
 */
 public static FieldSet create(Properties properties) {
 final Set<String> tokenNamesSet = properties.stringPropertyNames();
 final int numberOfTokens = tokenNamesSet.size(); 
 final String[] tokenNames = tokenNamesSet.toArray(new String[numberOfTokens]);
 final String[] tokenValues = new String[numberOfTokens];
 for (int tokenPosition = 0; tokenPosition < numberOfTokens; tokenPosition++) {
 String tokenName = tokenNames[tokenPosition];
 tokenValues[tokenPosition] = properties.getProperty(tokenName);
 }
 return new DefaultFieldSet(tokenValues, tokenNames);
 }
}

My concern with this approach is in regards to unit testing. For example, I have the following method (in another class) that needs to be unit tested:

public class CarFieldSetMapper extends BeanWrapperFieldSetMapper<Car> {
 @Override
 public Car mapFieldSet(FieldSet fieldSet) throws BindException { 
 Properties fieldProperties = fieldSet.getProperties();
 fieldProperties.put("modelDescription", fieldProperties.get("model"));
 removeDummyIndicator(fieldSet, fieldProperties);
 // build field from properties derived / transformed from the original field set
 FieldSet domainObjectFieldSet = PropertiesFieldSetFactory.create(fieldProperties);
 return super.mapFieldSet(domainObjectFieldSet);
 }
 private void removeDummyIndicator(FieldSet fieldSet, Properties fieldProperties) {
 // dummyIndicator is not a field of any domain object
 final String dummyIndicatorTokenName = "dummyIndicator";
 final String isDummyIndicator = fieldSet.readString(dummyIndicatorTokenName);
 if (isDummyIndicator.equalsIgnoreCase("Y")) {
 fieldProperties.put("model", "");
 }
 fieldProperties.remove(dummyIndicatorTokenName);
 }
}

I do not know of a great way to prevent PropertiesFieldSetFactory.create() from being called when running the unit test ala http://misko.hevery.com/2008/12/15/static-methods-are-death-to-testability/. Also, I'm really trying to extend the DefaultFieldSet functionality so I question using a factory. Note: On the other hand, Joshua Bloch seems to promote use of static factory methods in Effective Java (Chapter 2: Item 1).

Should I change PropertiesFieldSetFactory to PropertiesFieldSet extending DefaultFieldSet? If so, how can I set the FieldSet tokens and names. Both fields are private on the DefaultFieldSet without any way to set the values other than during construction.

Edit: Added code for removeDummyIndicator and correction of line fieldProperties.put("model", fieldProperties.get("model")); to fieldProperties.put("modelDescription", fieldProperties.get("model"));

• janos: Now you can +1 ;-)

Answer 1

The real issue here is that you do not need a PropertiesFieldSetFactory. Take a look at how you currently use it:

@Override
public Car mapFieldSet(FieldSet fieldSet) throws BindException { 
 Properties fieldProperties = fieldSet.getProperties();
 fieldProperties.put("modelDescription", fieldProperties.get("model"));
 removeDummyIndicator(fieldSet, fieldProperties);
 // build field from properties derived / transformed from the original field set
 FieldSet domainObjectFieldSet = PropertiesFieldSetFactory.create(fieldProperties);
 return super.mapFieldSet(domainObjectFieldSet);
}

This code is taking as input the FieldSet that was tokenized by the LineTokenizer you are using. It adds car-specific values to the Properties of that FieldSet, removes a value in case a dummy parameter is set, and reconstructs a FieldSet back from those properties. Those hoops are needed because FieldSet is immutable.

But look at this again: you don't need to fiddle with the FieldSet, only to have everything set-up automagically by the BeanWrapperFieldSetMapper. Store the result of super.mapFieldSet (invoked with the fieldSet given to the mapper) to a local car and do the necessary logic on that car:

public class CarFieldSetMapper extends BeanWrapperFieldSetMapper<Car> {
 @Override
 public void afterPropertiesSet() throws Exception {
 setStrict(false); // <-- ignore the non-existant "dummyIndicator"
 setTargetType(Car.class);
 super.afterPropertiesSet();
 }
 @Override
 public Car mapFieldSet(FieldSet fieldSet) throws BindException { 
 Car car = super.mapFieldSet(fieldSet);
 car.setModelDescription(car.getModel());
 if (fieldSet.readString("dummyIndicator").equalsIgnoreCase("Y")) {
 car.setModel(null);
 }
 return car;
 }
}

The Spring Batch approach to reading a file is:

• Take a LineMapper which is supposed to read a line and map it into your domain objects.
• This mapper tokenizes the line into a FieldSet with a LineTokenizer and maps this FieldSet into your domain object with a FieldSetMapper. As you can see, it is the tokenizer role to make a FieldSet out of the line, which is then used by the mapper to create the final domain object.
• By default, the line tokenizer creates FieldSet instances using a FieldSetFactory. It gets the names you configured and the values come from the parsed line.

So once the FieldSet has been constructed by the LineTokenizer, it should not be re-created: it contains the data as specified in the line that was read. This data is then transformed into your domain object; hence the only place to implement domain-specific logic is in the mapper, directly with the instance to be returned, and not in the FieldSet.

• James: I've included the missing method for removeDummyIndicator and correction to the fieldProperties.put("model", fieldProperties.get("model")); line. (Sorry about that.)
• Tunaki: @James I edited with a possible approach. The idea is still the same: create the fieldset from the linetokenizer only.
• James: That's great. I would give +1 but I'm new to Code Review and don't have the rep yet. The idea to move the Properties put / remove work from a FieldSetMapper to a LineTokenizer is perfect and makes a lot of sense. But what about the code of the PropertiesFieldSetFactory? Its code is fairly generic in nature (independent of LineTokenizer) and I will have other code that will need to convert Properties to token values and names. I feel like that we just shifted that problem to another class (i.e. from CarFieldSetMapper to FieldSetPropertiesLineTokenizer).