How I Use Transfer - Part V.1 - A Comment and Response
Posted At : July 8, 2008 10:28 AM | Posted By : Bob Silverberg
Related Categories: OO Design, How I Use Transfer, ColdFusion, Transfer
Brain Kotek wrote a lengthy and informative comment to my last blog post. I started composing a response as a comment of my own, but it got very long, and I think that both Brian's comment and my response contribute greatly to the series, so I've turned my response into a blog post. I'm including Brian's comments in quotes throughout this post.
Bob, great series so far! I had a couple of comments and opinions about what you're doing here that I wanted to bring up. I may create a blog entry on it if you or anyone else would like me to expand, but in brief:
Hi Brian, thanks for the comments. I think that most of your points are excellent, and will probably result in changes to the way I'm doing some things. Part of why things are the way they are is that this has been an organic process for me. When I started trying to write a service object I realized that it was pretty generic, so I started parameterizing things. I wasn't really thinking much about OO principles, rather just strictly trying to write as little code as possible.
I'm curious about how much benefit you feel you get by having to specify the name of a specific Transfer object in your ColdSpring config. I mean, you already know that this is a "ReviewService", and you've said that you may need to interact with other Transfer objects by name, so this mixing of a dynamically supplied object name with hardcoded object names would seem to be making things more complex than they might need to be.
I do find that I get quite a bit of benefit from having a "main" transfer object defined to my service. I end up having to write a lot less code because most of the operations for that object are dealt with via the AbstractService. I could easily move that info from Coldspring into a hardcoded variable in my Concrete Service, which would eliminate the mixing to which you refer. Would you see that as an improvement?
I have also thought, as I was writing this, that maybe I should move from inheritance to composition, and inject a "Modifier" object (for lack of a better name) into my service, which would basically do the same thing (the Modifier would have the same methods as the AbstractService).
My take is that the service layer should be less about wrapping a single Transfer object or database table and more about creating a useful external API for your applications to use.
Interesting point, so you don't see the service layer as "doing much", is that right? You see it more as a facade? It's sounding like much of what I put into the service layer you'd put into a gateway. N'est pas?
My opinion would also be that this service is too dependent on Transfer. I try to make my services as "dumb" as possible. This ReviewService is tightly coupled to Transfer and its implementation. Even the method signature is exposing the fact that Transfer is in use with the needsClone argument, meaning stuff "outside" of the service has to "know" about Transfer and under what conditions a clone is required.
I try to keep the database and Transfer-specific stuff down in a Gateway object. Gateways are meant to encapsulate interaction with an external resource, which is just what a database is. That way, if I ever need to switch out the ORM, or remove Transfer completely, I ideally only need to modify one layer of the application (the Gateways).
I definitely see your point about the service being too dependent on Transfer. As I was writing this I actually thought to myself, "Should I move the TQL stuff to a gateway?". I guess ultimately that would mean moving the guts of the Get() method from the service to the gateway, which, come to think of it, is a very interesting idea :-)
As for the needsClone argument, I'm starting to rethink that one. I thought that I needed a way to tell the service, which is the API, whether or not I need a business object that is going to be manipulated. If I word it that way (rather than referring to a clone) it no longer seems to be so tied to Transfer ;-) But that stems from the way I've implemented the get() method, which is used, in turn, by other methods. It's because of that design that I needed this needsClone argument, so maybe that is not the best design. The wheels are turning...
Lastly, as big a fan as I am of Transfer, I'm less a fan of TQL. It looks to me like the same thing could be done with normal SQL in about half the lines. Since it looks like you prefer the TQL approach, I'd be interested to hear your thoughts on its use compared to straight SQL.
Honestly, when I started using Transfer I just figured that I'd try to use all of its features to learn it well. The only advantage that I see currently is that it provides cross database support, but even that is questionable as a simple select statement can probably be written in a way that will run on most databases. I believe that in the future Mark is considering adding caching to these types of queries, which may yield another benefit. Other than that I don't really see any reason to use it, so I may give up on it at some point.
On a parallel note, it appears that you're enforcing some sort of security here within the service by limiting the review by the userID. Security can always be a difficult problem to tackle, but I wondered if you had considered any alternate methods of enforcing your security rules, such as a ColdSpring AOP advice?
I agree that it's always difficult to figure out how and where to enforce security. I haven't really considered many other options as in this case it seems to make sense and work here. Again, as I was writing this I did think that that rule really belonged in my Business Object, but that didn't fit in with my current design. If the rule needs to be enforced in order to return the correct business object, how can the rule exist _inside_ the business object? I guess I could start with an empty business object and then ask it to get itself from the database. Hmm, that sounds like an interesting twist.
The other issue here is that I'm not just enforcing security, I'm using logic to determine which object to return. It's not just that a user should only be able to edit their own review, it's also the fact that I'm trying to get a user's review without having the primary key of that review - just the Product and User, so I don't really see this as strictly a security concern.
I have yet to use Coldspring AOP for anything, so perhaps it would be a good candidate. My understanding of AOP is quite limited, but I thought that because this logic is specific to this one object and this one method, it wouldn't make a lot of sense to use AOP. I'd be interested to hear how you'd address this requirement with AOP.
So to summarize the main ideas I've gleaned from your comments:
- Think about moving logic from the service into the gateway.
- Think about moving logic from the service into business objects.
Interesting that those are strategies that I discussed in the second article. I guess sometimes it's easier to see things from an outside perspective. Thanks again for the excellent comments. I look forward to more of them.
In regard to the Transfer object being specified through ColdSpring, the real issue that I saw was that at first glance it seems to enforce the prevailing tendency to create the whole "DAO/Bean/Gateway/Service for every database table".
Since most of what you're doing in your AbstractService seems like it might be better placed into an AbstractGateway, the issue sort of moves to specifying it to the Gateway. Which might make more sense, but it still leaves a lingering "code smell" to me that one "special" variable name (the Transfer object) is specified dynamically, while any other Transfer objects or table names are still hardcoded. Again, if you have a ReviewGateway, you already know that its a "review" gateway.
That said, if enough of your Gateway methods are specific to one Transfer object, and enough of the methods are handled in a generic way by methods in AbstractGateway, I can't say that specifying the primary object name in ColdSpring or in your object's constructor is a bad idea. I'd probably go the route of just specifying it in the constructor. It's not like your ReviewGateway is going to need to be dynamically configured to suddenly use some other Transfer object like "Permission".
I'm not sure what you mean by the Modifier object though. Would that have the Gateway methods in it? Or do mean it would be a factory or populator that would take data and create a Transfer object and populate it?
In general I'm not a fan of generic method names like "get()". I'm one of the people who likes my API to be very clear and my method names to be as self documenting as possible. So even if you have a generic get() method in AbstractGateway, I'd still give the concrete method a more explicit name like getUserReview() or something. Internally it can call the generic get() method in the superclass, but someone looking at the gateway call would probably have a better idea of what it will do if the method name is more clear. Again, just my opinion.
I honestly haven't messed around much with TQL so I may well be missing some killer use of it. I also had worked with Reactor before, which also had an OO-based query syntax, and I gave up on it because it got so complex so fast just to do something that SQL does in half the code. :-)
Security is definitely a place where there are many, many possible ways to do things. My approach is that things like security rules are really not the concern of the business object. Security rules are business rules, and the same object could be used in more than one context or application, where the rules that need to be applied may differ. That's why I prefer to do it at the service level if necessary, and in AOP advices where possible.
Security is really a "cross-cutting" concern, like logging, and is ripe for an AOP solution. However, AOP can be difficult to wrap one's head around, and depending on the security that needs to be enforced, may be too difficult to implement in AOP.
I've taken an approach where I create advices that I can attach metadata to (as XML), so that it can look at what method is being called and what rules are specified for that method, and the advice will respond accordingly. To be fair though, this was not an easy solution to implement. I plan to release my updated ColdSpring MetadataAwareAdvice and my MetadataAwareProxyFactoryBean as part of my RIAForge ColdSpring Utilities project, which might shed some light on how this could work. Maybe this will nudge me to take the time to get that out. ;-)
Regards,
Brian
1. It makes sense to move much of the logic out of my service and into a corresponding gateway. This will allow me to decouple my service from Transfer.
2. The current Get() method in the AbstractService can be moved into a BusinessObjectFactory, again decoupling the service from Transfer.
3. Remove the TransferClassName and EntityDesc from Coldspring and just hardcode them into the constructor of the Concrete Service Objects. As the values are not likely to ever have to change, there's really no reason to have them managed by Coldspring.
4. I'm going to keep my method names. I'll stick with Get() instead of moving to getReview() and getProduct(). It means less to maintain and less to change if the API to Get() ever changes.
5. Regarding the question of why TQL, I did come up with an additional idea of a benefit:
If you use TQL you can actually query your object model. So if you build your object model first, and then simply come up with a db that will support it, and use an ORM as the middle man, then you could, theoretically change the structure of the db any way you want but your code could stay the same. It's like abstracting your db schema right out of the app. I'm not sure that I'd ever benefit from this, but it _is_ a theoretical benefit.
I don't think I'm going to implement any of these changes immediately. I'd rather get through the series first, and then go back and refactor based on all of the feedback that I get, and then maybe there will be a follow-up series.
http://www.mattwoodward.com/blog/index.cfm?event=s...
In terms of how I'm using my Gateway Objects, I am using them to encapsulate database access, so I am trying to move any and all database access into them, regardless of whether it deals with a single row or multiple rows. Because Transfer creates my Business Objects for me, and those Business Objects have the capability of a standard DAO inside them, my gateways do usually end up consisting mostly of methods that access multiple rows, but I don't feel that that is a requirement.
I didn't read all of the related posts that Matt refers to in his article, but I believe that I agree with his sentiment about not worrying too much about semantics and instead spending your time worrying about what works for you, and why.