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

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"));

• Now you can +1 ;-) – janos Jul 1 '16 at 12:57

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());
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.

• I've included the missing method for removeDummyIndicator and correction to the fieldProperties.put("model", fieldProperties.get("model")); line. (Sorry about that.) – James Jun 29 '16 at 20:42
• @James I edited with a possible approach. The idea is still the same: create the fieldset from the linetokenizer only. – Tunaki Jun 29 '16 at 21:18
• 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). – James Jun 29 '16 at 21:39