Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FieldSetFactory inconsistent parameter ordering create method [BATCH-1662] #1922

Closed
spring-issuemaster opened this issue Nov 26, 2010 · 4 comments

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented Nov 26, 2010

Stefan Larsson opened BATCH-1662 and commented

The parameter ordering of the interface method FieldSetFactory.create(String[], String[]) is inconsistent with it's DefaultFieldSetFactory implementation and how it's used in AbstractLineTokenizer.

The combinations are:
FieldSetFactory: FieldSet create(String[] names, String[] values);
DefaultFieldSetFactory: public FieldSet create(String[] values, String[] names) {
AbstractLineTokenizer: return fieldSetFactory.create(values, names);

Somewhat related argument ordering usage:
DefaultFieldSet two-argument constructor: public DefaultFieldSet(String[] tokens, String[] names)

While it normally might not be a good idea to change interface definitions but the other code, perhaps it's a good idea in this specific case in order to be compatible with existing code which does interact with AbstractLineTokenizer or DefaultFieldSetFactory? It appears that the FieldSetFactory interface is deviating from conventions everywhere else.


Affects: 2.1.5

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Nov 26, 2010

Dave Syer commented

So this is really a documentation issue with FieldSetFactory? Is that it?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Nov 26, 2010

Stefan Larsson commented

My suggestion would be to modify FieldSetFactory, just change
FieldSet create(String[] names, String[] values);
into
FieldSet create(String[] values, String[] names);

As for documentation, the interface methods lack any javadoc, however the method names and arguments are rather self-explanatory in this case.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Nov 26, 2010

Dave Syer commented

OK, so that's just a name change (which amounts to documentation really). I added some Javadocs as well, thanks.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Feb 9, 2011

Stefan Larsson commented

The fix in 2.1.6 looks good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.