Menu
▾
▴
springframework-developer
|
From: <pr...@se...> - 2003-07-15 20:13:56
|
Before I comment, thanks for all your work on the demo Ken. I haven't been able to contribute to the project for a few months, and due to work/family commitments, I probably won't be able to start contributing again until September. I simply mention this so you can evaluate my comments accordingly (I value action more than talk, and right now you're acting, I'm just talking :) ). >>I have just commited my latest changes to Petclinic. They include >>the following changes: >>- ClinicImpl, ClinicDAO, and ClinicJdbcDAO have been replaced >>by AbstractJdbcClinic, HsqlClinic, and MysqlClinic. I question why you would make this change? I understand (and like) the Hsql and MySql implementations since they show the core of what Spring allows (that you can make specific implementations where required). However, you are coupling the implementation of clinic with the dao interface and the jdbc dao implementation. I may be missing something, but isn't this a step backwards? In my mind, the implementation of Clinic should be seperate from its storage mechansim (ClinicDAO), and the DAO would be an interface which is then implemented (in this case by the above mentioned AbstractJDBC and then the 2 db-specific implementations. To quote Rod's book "sometimes we are unable to seperate the two (business logic and persistence logic)" but I don't see this as one of those exceptions. While I personally hate EJB, this new design totally excludes it (since there is no clinic object unless you use JDBC), but it also would exclude other persistence mechanisms (xml, flat-file, etc.) or even using an "unpersisted" version of the clinic (unless you choose to carry around all the JDBC stuff as baggage which adds 13 RdbmsOperation objects). Sorry to question your design choice, and maybe I'm totally missing something, just curious on what advantage this change brings. Trevor D. Cook |
|
From: <pr...@se...> - 2003-07-16 11:10:10
|
Your reasoning (and Rod/Juergen's) makes perfect sense, and a quick look at the source confirms the "bad smells" you mentioned. I know how a manager feels now, asking a question based on incomplete info (I commented based on the "design" I saw by the class names, and didn't actually dig into the code) that is usually a very well-thought out decision by the programmer. "The programmers always right" :) , I was just a little quick (and pointy-haired) for a few minutes last night! My biggest concern seeing those changes was that our samples will be used by many as a model of how to design their programs using Spring, and it may appear to many that a Spring best-practice is to merge the business object with the persistence object. While it makes perfect sense in this case (due to the reasons you identified), it is not totally clear why this decision was made by looking at the code. We should probably document why this code is done this way to help newbies understand that this a solution to this specific problem (and not necessarily the "normal" way for most apps). Documenting it will also prevent somebody (like me) who isn't familiar with the history and design decisions from changing it back to how it was before. Great work, Trevor |
|
From: Rod J. <rod...@in...> - 2003-07-16 12:10:59
|
> My biggest concern seeing those changes was that our samples will be used by > many as a model of how to design their programs using Spring, and it may > appear to many that a Spring best-practice is to merge the business object > with the persistence object. While it makes perfect sense in this case (due > to the reasons you identified), it is not totally clear why this decision was > made by looking at the code. We should probably document why this code is > done this way to help newbies understand that this a solution to this specific > problem (and not necessarily the "normal" way for most apps). Documenting it > will also prevent somebody (like me) who isn't familiar with the history and > design decisions from changing it back to how it was before. Good point. We should also make it clear that Spring is _perfect_ for implementing the DAO pattern. Rod > > > Great work, > Trevor > > > > > ------------------------------------------------------- > This SF.net email is sponsored by: VM Ware > With VMware you can run multiple operating systems on a single machine. > WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the > same time. Free trial click here: http://www.vmware.com/wl/offer/345/0 > _______________________________________________ > Springframework-developer mailing list > Spr...@li... > https://lists.sourceforge.net/lists/listinfo/springframework-developer > |
|
From: Ken K. <kk...@kk...> - 2003-07-16 16:44:46
|
Thomas, I'll address your concern in the tutorial narrative. I suppose I ought to put some notes in the code too. Thanks again for your probing comments. Ken pr...@se... wrote: >Your reasoning (and Rod/Juergen's) makes perfect sense, and a quick look at >the source confirms the "bad smells" you mentioned. I know how a manager >feels now, asking a question based on incomplete info (I commented based on >the "design" I saw by the class names, and didn't actually dig into the code) >that is usually a very well-thought out decision by the programmer. "The >programmers always right" :) , I was just a little quick (and pointy-haired) >for a few minutes last night! > >My biggest concern seeing those changes was that our samples will be used by >many as a model of how to design their programs using Spring, and it may >appear to many that a Spring best-practice is to merge the business object >with the persistence object. While it makes perfect sense in this case (due >to the reasons you identified), it is not totally clear why this decision was >made by looking at the code. We should probably document why this code is >done this way to help newbies understand that this a solution to this specific >problem (and not necessarily the "normal" way for most apps). Documenting it >will also prevent somebody (like me) who isn't familiar with the history and >design decisions from changing it back to how it was before. > > >Great work, >Trevor > > > > >------------------------------------------------------- >This SF.net email is sponsored by: VM Ware >With VMware you can run multiple operating systems on a single machine. >WITHOUT REBOOTING! Mix Linux / Windows / Novell virtual machines at the >same time. Free trial click here: http://www.vmware.com/wl/offer/345/0 >_______________________________________________ >Springframework-developer mailing list >Spr...@li... >https://lists.sourceforge.net/lists/listinfo/springframework-developer > > > > |
|
From: Rod J. <rod...@in...> - 2003-07-15 20:47:50
|
> I question why you would make this change? I understand (and like) the Hsql > and MySql implementations since they show the core of what Spring allows (that > you can make specific implementations where required). However, you are > coupling the implementation of clinic with the dao interface and the jdbc dao > implementation. I may be missing something, but isn't this a step backwards? > In my mind, the implementation of Clinic should be seperate from its storage > mechansim (ClinicDAO), and the DAO would be an interface which is then > implemented (in this case by the above mentioned AbstractJDBC and then the 2 > db-specific implementations. To quote Rod's book "sometimes we are unable to > seperate the two (business logic and persistence logic)" but I don't see this > as one of those exceptions. While I personally hate EJB, this new design > totally excludes it (since there is no clinic object unless you use JDBC), but > it also would exclude other persistence mechanisms (xml, flat-file, etc.) or > even using an "unpersisted" version of the clinic (unless you choose to carry > around all the JDBC stuff as baggage which adds 13 RdbmsOperation objects). Without going into the specifics, which I don't have time to think about right now, I should say that I've changed my opinions somewhat since the book regarding separating persistence & business logic. Where transparent persistence mechanisms such as JDO or Hibernate are concerned, I think there's no reason not to put business logic in persistent objects, as the persistent objects are not polluted by persistence-mechanism specific code. Entity beans have lumbered us with some baggage in this area. With JDBC, separation via a DAO does make more sense, as otherwise the JDBC code can pollute the object model. However in the case of the PetClinic I seem to recall there being a high level of duplication between DAO and business objects. In this case, simple subclassing to handle the persistence details in the subclasses probably makes more sense. Regards, Rod |
|
From: Ken K. <kk...@kk...> - 2003-07-16 03:50:20
|
Trevor, I'm really glad you asked :-) . Really, I do appreciate your questioning this design decision and truly welcome more criticism. It helps me learn. I first got the suggestion from Juergen: <Juergen> Regarding the implementation: I've noticed that the Clinic interface duplicates ClinicDAO's methods to a large extent, and that ClinicImpl adds cross-referencing and caching to the entities (data access aspects). This isn't really proper separation between data access code and business logic, but of course the borders are often somewhat blurring. In our case, it's probably more appropriate to merge the two, i.e. offer just a Clinic interface, with the common logic in an AbstractClinic base class, and a ClinicJdbcImpl default implementation. IMO, the app simply isn't complex enough for separated business and data access layers. The more important thing is clear separation between the business and the web layer: Those two should never be merged, not even in very simple apps. </Juergen> These comments really hit home because I was already not liking the smell of that duplication in the Clinic and ClinicDAO interfaces, but I really didn't understand at the time why this should be so. When Rod seconded this opinion, I agreed to go ahead and do it, although I was still a bit unsure about it. While doing the refactoring, It finally dawned on me as to why this approach makes sense for this app. The situation really became clear to me while I was writing the new TestCase for Owner (+1 for XP). The application is all about database access and there is very little business logic in the application outside of that. What few business rules there are have been implemented by the Validators and the Owner class. Therefore, this change doesn't really exclude other options as you can just provide other classes that implement the Clinic interface, i.e. XmlClinic, EJBClinic, MockClinic or whatever. Since AbstractJdbcClinic doesn't really implement any business logic besides persistence, there is no need to derive from it except for Jdbc purposes. The reason why I didn't implement an AbstractClinic class as Juergen had suggested was simply because I couldn't find anything for it to do. I think the advantage that this change brings is that is simpler and clearer and as such suits the tutorial purpose just a little bit better. There actually is no clear and strong advantage to either implementation choice for this situation, IMO. Regards, Ken pr...@se... wrote: >Before I comment, thanks for all your work on the demo Ken. I haven't been >able to contribute to the project for a few months, and due to work/family >commitments, I probably won't be able to start contributing again until >September. I simply mention this so you can evaluate my comments accordingly >(I value action more than talk, and right now you're acting, I'm just >talking :) ). > > > > >>>I have just commited my latest changes to Petclinic. They include >>>the following changes: >>>- ClinicImpl, ClinicDAO, and ClinicJdbcDAO have been replaced >>>by AbstractJdbcClinic, HsqlClinic, and MysqlClinic. >>> >>> > >I question why you would make this change? I understand (and like) the Hsql >and MySql implementations since they show the core of what Spring allows (that >you can make specific implementations where required). However, you are >coupling the implementation of clinic with the dao interface and the jdbc dao >implementation. I may be missing something, but isn't this a step backwards? >In my mind, the implementation of Clinic should be seperate from its storage >mechansim (ClinicDAO), and the DAO would be an interface which is then >implemented (in this case by the above mentioned AbstractJDBC and then the 2 >db-specific implementations. To quote Rod's book "sometimes we are unable to >seperate the two (business logic and persistence logic)" but I don't see this >as one of those exceptions. While I personally hate EJB, this new design >totally excludes it (since there is no clinic object unless you use JDBC), but >it also would exclude other persistence mechanisms (xml, flat-file, etc.) or >even using an "unpersisted" version of the clinic (unless you choose to carry >around all the JDBC stuff as baggage which adds 13 RdbmsOperation objects). > >Sorry to question your design choice, and maybe I'm totally missing something, >just curious on what advantage this change brings. > >Trevor D. Cook > > >------------------------------------------------------- >This SF.Net email sponsored by: Parasoft >Error proof Web apps, automate testing & more. >Download & eval WebKing and get a free book. >www.parasoft.com/bulletproofapps1 >_______________________________________________ >Springframework-developer mailing list >Spr...@li... >https://lists.sourceforge.net/lists/listinfo/springframework-developer > > > > |
