Martin Mucha has posted comments on this change. Change subject: webadmin: removed caching from EnumTranslator. ......................................................................
Patch Set 3: thanks for answers. I think I'll opt for strict separation of anyhow related/unrelated just to eliminate possibility of work done in vain. About Translator class. Your observation is correct. There is NOT any interface for translator and there definitely should be one if we ever want to use DI more seriously or if we want better testability. Actually this implementation is more like a stub one intended to be used in testing and if we think about some class implementing toString() in a way which just returns null, the containsKey() is wrong anyway, since in this case we HAVE translation for that key, only it's result is just a 'null'. OK. I'd propose further steps. To be consistent with formerly said, I'd leave this Translator class as it is, since this patch is fixing caching not another, although real, problem(s). Next I'd drop implementation of Translator since it's meaningless && not-used anyway, and make an interface out of it, both in separate commit. Agreed? -- To view, visit http://gerrit.ovirt.org/26048 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3df424d8e7a35ed249b2760da79deba7db31b785 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@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
