61 input location #62
Conversation
| */ | ||
| public static interface InputLocationBuilder | ||
| { | ||
| Object toInputLocation( XmlPullParser parser ); |
There was a problem hiding this comment.
What about 'getCurrentInputLocation'?
Should we explicitly state the expected behavior?
Is it better to return an GenericInputLocation, just as a placeholder interface? It will make code more typesafe and clearer IMHO
There was a problem hiding this comment.
'getCurrentInputLocation': since it is expected to create an object, I don't like 'get'
What do you mean by "state the expected behavior"?
Since the implementation is really something that will be specific to the user, and that the user will have to cast, I don't see the benefit of creating an empty interface
There was a problem hiding this comment.
'getCurrentInputLocation': since it is expected to create an object, I don't like 'get'
My concern is that the function takes a stateful object as parameter, so I image it will return a view of the current state of the parser.
the word 'to' seems to me like a 'conversion'.
Not so important to me, I am very new to this code.
What do you mean by "state the expected behavior"?
For instance, if the method is called twice is it expected to return two equivalent objects, the same object, or what ?
Object a = toInputLocation( parser );
Object b = toInputLocation( parser );
should b == a, b.quals(a) or it is not important ?
Since the implementation is really something that will be specific to the user, and that the user will have to cast, I don't see the benefit of creating an empty interface
Fine
There was a problem hiding this comment.
for input location, ordering is not important: really, the documentation to read is https://codehaus-plexus.github.io/modello/location-tracking.html
|
yes, reviewing and seconding is a good opportunity to learn: thank you for your review |
| @@ -86,6 +88,12 @@ public Xpp3Dom( String name ) | |||
| childMap = new HashMap<String, Xpp3Dom>(); | |||
| } | |||
|
|
|||
| public Xpp3Dom( String name, Object inputLocation ) | |||
There was a problem hiding this comment.
Thinking out loud: Can we add a generic to Xpp3Dom, like Xpp3Dom<T>, where T reflects the inputLocation?
There was a problem hiding this comment.
I suppose it is feasible: IMHO, gives too much visibility to input location tracking, which is really something very special, but feasible
hboutemy
force-pushed
the
61-input_location
branch
from
997f704
to
0eaf19a
Compare
see #61