Martin Mucha has posted comments on this change. Change subject: core: Introduce BusinessEntityMap ......................................................................
Patch Set 13: (1 comment) https://gerrit.ovirt.org/#/c/33329/13/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BusinessEntityMap.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/BusinessEntityMap.java: Line 19: entitiesByName = new HashMap<String, E>(); Line 20: entitiesById = new HashMap<Guid, E>(); Line 21: Line 22: for (E e : entities) { Line 23: if (e.getName() != null) { > I can't. Here there is a good chance that either the name or the id are nul I dont understand this explanation. Entites.entitiesByName resp. businessEntitiesById assumes without check, that there's no duplicates among keys. Which may be right for certain situations, but will yield wrong result if violated. This also relates to null valued keys (name/id). Null valued name/id is just fine, but it will not identify record if there are more such records. So if something, 'Entities' methods should be fixed to handle duplicates and entities with null valued keys, which I do believe should be sorted out here as well. If null valued keys is alright here (doubt so), mapping methods should be introduced here not elsewhere. Line 24: entitiesByName.put(e.getName(), e); Line 25: } Line 26: Line 27: if (e.getId() != null) { -- To view, visit https://gerrit.ovirt.org/33329 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4540f813f8a787a38ba3e692b6ddd3fcc6be536a Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <wallaroo1...@gmail.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
