Tomas Jelinek has posted comments on this change.

Change subject: userportal,webadmin: type ahead list box
......................................................................


Patch Set 8:

sry, wrong formatting, this time with better:

> This was just a summary, please refer to my numerous inline 
> comments for context. 

I have answered to all of them - do you have any more concerns?

> I don't see that there's that much shared code between the two classes 

Really? Look at ListModelSuggestBox - all that deleted code is shared between 
the two children of BaseListModelSuggestBox.

> and I see a lot of difference in functionality (the only logical connection 
> is that of delegating string completion to a SuggestBox, which has 
> nothing to do with ListBoxSuggestBox). 

Both are some special case of suggest box. You can not have better case for 
inheritance. It is like saying that "dog and a cat can not have a common parent 
animal as the dog and the cat are so different in animals".

> Most of the code that's shared is trivial delegation of keyboard event 
> handlers, etc. 

sure, typical boilerplate you want to write only once

> Rewriting the code of a class to fit a different class isn't code sharing. 

There are two different implementations of a suggest box. They have a common 
parent (as both are suggest boxes). And the common code (~180 lines) is in the 
common parent. Do you see any problem with this design?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I956af3c675894c850a1a104a81cec49f4bd62011
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to