Martin Mucha has posted comments on this change.

Change subject: core: Remove code duplication in VdsNetworkInterface.toString()
......................................................................


Patch Set 1:

it's ok to remove duplicates and do common only on parent.

But there's still duplicates in forming that to string and too much trust that 
‘everybody’ will cooperate nicely in forming that string. 

We can fix it by own code (ie. method accepting list pairs fieldName-fieldValue 
responsible for creating output string) or by using 
org.apache.commons.lang3.builder.ToStringBuilder. That way 
‘serializeAdditionalAttributes’ have to be changed to accept builder and use it 
to ‘add’ data to it instead of creating substring hoping it will fit in 
enclosing one. Using this builder we move responsibility of forming string to 
builder, select one of supported tostring styles and (aside from simpler code) 
we always end up with non-cripled toString text.

also it's worth to think what are these toStrings used for. Seeing it's 
verbosity I can think only about logging, where it should not be problem to log 
*everything* from this class and reflection variant of tostring builder could 
be used.

-- 
To view, visit http://gerrit.ovirt.org/36215
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I94bbf7113e848f6ca8ae4a38e46303050d653135
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Martin Mucha <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to