Vojtech Szocs has posted comments on this change.

Change subject: webadmin: Sort NIC column in Network/Hosts subtab
......................................................................


Patch Set 10: Code-Review+1

Giving +1 but I saw Gilad had some concerns so we should wait for his feedback.

As for LexoNumericComparator vs. Nameable integration, I'd suggest to make a 
separate patch like this:

 public abstract class LexoNumericComparatorBase<T> implements Comparator<T>, 
Serializable {

     ...

     @Override
     public int compare(T o1, T o2) {
         return compare(getString(o1), getString(o2));
     }

     // This method is now private
     private int compare(String str1, String str2) {
         return comp(str1, str2, caseSensitive);
     }
  
     protected abstract String getString(T obj);

     ...

 }

This way we can easily support anything that is -or- can become a String, for 
example:

 public class LexoNumericComparator extends LexoNumericComparatorBase<String> {
     @Override
     protected String getString(String obj) {
         return obj;
     }
 }

 public class NameableLexoNumericComparator extends 
LexoNumericComparatorBase<Nameable> {
     @Override
     protected String getString(Nameable obj) {
         return obj.getName();
     }
 }

Lior/Gilad, what do you think?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie52ec4faafbb06a6cd62b988edc11d32db3bb765
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to