Refining Uncle Bob’s Clean Code（二）

Looking back at the Args example, i would see class Args as the root of its ‘application’, thus responsible for bringing the different parts into life and statisfying their needs. Under this view, Args is nothing more than a facade which delegates the different tasks to their collaborators:

01	import static java.util.Arrays.asList;

02

03	public class Args {

04

...

05

06	private Marshalers argumentMarshalers = null;

07

08	public Args( String schema, String[] args) throws ArgsException {

09	argumentMarshalers = Marshallers.Factory.getMarshalersFor( schema );

10	ArgumentPopulator.distribute( asList(args) ).to( argumentMarshalers );

11

}

12

13

...

14

15

}

As you can see, the result of the schema parsing (that is the found ArgumentMarshalers by Marshallers.Factory, each responsible for every single found argumentId) acts as input for the parsing / population of the arguments. Therefore, this second action only needs to know about the current arguments and the responsible Marshalers which will receive the related argument values they have to convert.

Boundaries

As you may have seen in the above piece of code, we no longer rely on a Map when handling ArgumentMarshalers. Like said in Chapter 8 ‘Boundaries‘, Provider of third party packages ‘strive for broad applicability. Users on the other side are looking for an interface that’s focussed on their particular needs‘.

Further on, Ron Jeffries mentioned in Chapter 1, that most programs include handling collections (the marshalers in our case). He also suggested to come up with an own collection type for specific items, so that you’re able to provide a ‘domain specific’ interface, related to the specific needs of usage.
That said, we could encapsulate the Set of responsible Marshalers in an own collection (say Marshalers), only providing domain specific Methods (like isMarshallerAvailabeFor( String argumentId )), but prohibit the possibility to clear the collection for example:

01	import java.util.ArrayList;

02	import java.util.HashMap;

03	import java.util.List;

04	import java.util.Map;

05

06	public class Marshalers {

07

08	private Map<String,ArgumentMarshaler> argumentIdMarshalers = null;

09

10	private Marshalers( List<ArgumentMarshaler> marshalers ){

11

12	this.argumentIdMarshalers = new HashMap<String, ArgumentMarshaler>();

13

14	for( ArgumentMarshaler marshaler : marshalers ){

15	argumentIdMarshalers.put( marshaler.getArgumentId(), marshaler );

16

}

17

}

18

19	public boolean isMarshalerAvailabeFor( String argumentId ){

20	return argumentIdMarshalers.containsKey( argumentId );

21

}

22

23	public ArgumentMarshaler getMarshalerFor( String argumentId ) throws ArgsException{

24	return argumentIdMarshalers.get( argumentId );

25

}

26

27	public int size(){

28	return argumentIdMarshalers.size();

29

}

30

}

As you can see, java.util.Map is now nothing more than an implementation detail, not showing up at the interface of the Marshalerscollection.

Power to the ArgumentMarshaler

One thing that surprised me most, is the if/else if/else chain within method parseSchemaElement() in order to select an appropriate ArgumentMarshaler for a given argumentId (schema element).
Again, i wonder if Args should be responsible to bound a specific argumentId to a specific ArgumentMarshaler, or if this shouldn’t be in the responsibility of the single Marshaler.

I would rather like to have the possibility of asking a Marshaler, if he’s able to handle a given schema element, so that the Marshaler can decide of its own if he wants to accept a given schema element. This would mean, that we could get rid of the if/else if/else chain by only iterating the registered Marshalers until there’s a Marshaler found that feels responsible for the given element (replacing conditional with polymorphism).
Furthermore, parsing and separating the single schema elements (knowledge about the schema notation) and the ‘selection’ of an appropriate Marshaler is cleanly separated.

Since we already moved the accordant logic to Marshalers.Factory, let’s take a closer look at the new mechanism on how to identify an appropriate ArgumentMarshaler for a given schema element:

01	public class Marshalers {

02

03

...

04

05	public static class Factory{

06

07	private static final List<ArgumentMarshaler> registeredMarshalers =

08	new ArrayList<ArgumentMarshaler>();

09

10	public static void register( ArgumentMarshaler marshaler ){

11	registeredMarshalers.add( marshaler );

12

}

13

14	public static Marshalers getMarshalersFor( String schema ) throws ArgsException{

15	List<ArgumentMarshaler> marshalers = new ArrayList<ArgumentMarshaler>();

16

17	for( String element : schema.split( "," ) ) {

18	if( element.length() > 0 ) {

19	marshalers.add( marshalerFor( element.trim() ) );

20

}

21

}

22	return new Marshalers( marshalers );

23

}

24

25	private static ArgumentMarshaler marshalerFor( String element ) throws ArgsException {

26

27	for( ArgumentMarshaler marshaler : registeredMarshalers ){

28

29	if( marshaler.canHandle( element ) ){

30	return marshaler.newInstanceFor( element );

31

}

32

}

33	throw new ArgsException( ArgsException.ErrorCode.INVALID_FORMAT, element, element );

34

}

35

}

36

}

As you might see, we shifted some of the responsibility to ArgumentMarshaler. Now each Marshaler knows for himself, if he wants to handle a specific schema element. You might say, that the argument type postfix (like ‘*’ or ‘#’) now is hidden inside of each single Marshaler, but that’s only a question of configuration. As you will see, it would be easy to come up with a slide variation, where the argument type postfix could be passed as a constructor argument to each single marshaler implementation (that kind of configuration would fall in the responsibility of class Args, since it is responsible for the Setup like mentioned before).

Another point worth to mention: Factory is an inner class within Marshalers, so that it’s able to access the private constructor ofMarshalers for returning a new instance. Since the constructor is private, the only way to receive an instance of Marshalers is viaMarshalers.Factory.getMarshalersFor( String schema ).