Martin Mucha has posted comments on this change.

Change subject: core: wrapper of HashMap for counting number of objects
......................................................................


Patch Set 5:

(3 comments)

answers.

http://gerrit.ovirt.org/#/c/26402/5/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 35:     public boolean remove(T key) {
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.
> the code formatter adds a single space between the end-of-line to the comme
Done
Line 40:         }
Line 41: 
Line 42: 
Line 43:         int count = counter.decrement();


Line 38:             //it's not there, ignore;
Line 39:             return false;       //key was not there! So it was not 
removed.
Line 40:         }
Line 41: 
Line 42: 
> please remove the extra space line
Done
Line 43:         int count = counter.decrement();
Line 44:         if (count == 0) {
Line 45:             map.remove(key);
Line 46:             return true;        //key was removed;


Line 43:         int count = counter.decrement();
Line 44:         if (count == 0) {
Line 45:             map.remove(key);
Line 46:             return true;        //key was removed;
Line 47:         } else if (count < 0) {
> in which cases do you anticipate this condition to occur ? In a non-safe th
in unexpected ones. I don't care when/if it happens. Currently is, at least I 
think, impossible to produce this warning. But like some it guy said: 
"Impossible is next friday". So next friday you'll see in your code, that 
there's an error to be fixed. Sounds kind to me.

warning being too general: In log there will be name of class and message. This 
should be sufficient for 90 lines long class.
----
So what do you want?  Remove that extra logging altogether? Or to alter warning 
message somehow?
Line 48:             log.warn("count underflow.");
Line 49:             map.remove(key);
Line 50:             return true;        //key was removed.
Line 51:         }


-- 
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: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <[email protected]>
Gerrit-Reviewer: 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

Reply via email to