Vojtech Szocs has posted comments on this change. Change subject: userportal,webadmin: brand 404 error pages. ......................................................................
Patch Set 4: After discussing common code reuse (cross-cutting concerns) in servlet context with Alex, my original idea to implement code reuse via base class isn't that good since it enforces a servlet to inherit some base class, imposing restriction on class inheritance. Inheritance is good for conceptually related classes (i.e. abstraction vs. specialization for given concept/functionality) but it's not that good for conceptually un-related classes that have cross-cutting concerns (i.e. welcome servlet vs. page not found servlet both using branding). I guess a better option to implement cross-cutting concern (such as setting up branding-related stuff for different servlets) is to write a filter, just like LocaleFilter does today. Another alternative is to use AOP and replace filter with annotation + aspect that processes this annotation, but I still feel filters are a better option, just because filters can be applied to any web resource (via URL pattern), be it servlet or static file, i.e. they are decoupled from servlets and don't require any modification to existing servlet code. -- To view, visit http://gerrit.ovirt.org/18196 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3b0406ab6dea4a2cacebd4ad6e62c8bb7a0d44c0 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> 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
