Mike Kolesnik has posted comments on this change. Change subject: core: wrapper of HashMap for counting number of objects ......................................................................
Patch Set 6: (5 comments) http://gerrit.ovirt.org/#/c/26402/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/ObjectCounter.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/ObjectCounter.java: Line 25: return true; Line 26: } else if (allowDuplicate) { Line 27: counter.increment(); Line 28: return true; Line 29: } else { It's not necessary to have the else block. Line 30: return false; Line 31: } Line 32: } Line 33: Line 31: } Line 32: } Line 33: Line 34: Line 35: public boolean remove(T key) { We don't use comments succeeding code, please change to be before the line of code. Also with some good javadoc, most (if not all) the comments are unnecessary. Line 36: ModifiableInteger counter = map.get(key); Line 37: if (counter == null) { Line 38: //it's not there, ignore; Line 39: return false; //key was not there! So it was not removed. Line 49: map.remove(key); Line 50: return true; //key was removed. Line 51: } Line 52: Line 53: return false; //key is still present with lessened count. Not sure what is false meaning here.. Line 54: Line 55: } Line 56: Line 57: public boolean contains(T key) { Line 60: Line 61: private static class ModifiableInteger { Line 62: private int count; Line 63: Line 64: public ModifiableInteger(int initialValue) { > I'd add a default constructor that sets count to 1 You mean instead of this one? Line 65: setCount(initialValue); Line 66: } Line 67: Line 68: public int getCount() { Line 74: } Line 75: Line 76: public int increment() { Line 77: count++; Line 78: return count; Why do you need the return value? Line 79: } Line 80: Line 81: public int decrement() { Line 82: count--; -- To view, visit http://gerrit.ovirt.org/26402 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I38b9ead37a8ebfc56103b87c65ba582a84f4dda6 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Mike Kolesnik <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
