Moti Asayag has posted comments on this change.

Change subject: db: aggregate qos and storage qos impl
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.ovirt.org/#/c/27094/6/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/qos/BaseQosDaoFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/qos/BaseQosDaoFacadeImpl.java:

Line 12: import org.springframework.jdbc.core.namedparam.MapSqlParameterSource;
Line 13: 
Line 14: public abstract class BaseQosDaoFacadeImpl<T extends BaseQos> extends 
DefaultGenericDaoDbFacade<T, Guid> implements QosDao<T> {
Line 15:     private final QosType qosType;
Line 16:     RowMapper<T> mapper = createEntityRowMapper();
> I fine with that.
so let me see if i get this right:
 1. you don't want to implement toString() since creating an instance of 
StringBuilder is too expensive.
 2. You're fine with creating new instance of RawMapper every dao call.

 a. toString() is kind of convention in a lot of cases
 b. static mapper is kind of convention in a lot of cases

I'm a bit lost here and can't track your logic in selecting which convention to 
embrace and why. Especially when two claims contradict each other.
Line 17: 
Line 18:     public BaseQosDaoFacadeImpl(QosType qosType) {
Line 19:         super("qos");
Line 20:         this.qosType = qosType;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a9af59277b5055453159f002f19046c0051d63b
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Kobi Ianko <k...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.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

Reply via email to