Refactoring - How would you approach this?
James Buckingham
I've just throw a question out to our team about how they think they
would refactor this scenerio. While I wait for the answers to come
flooding back I thought I'd maybe ask the same question to people that
are more knowledge of this stuff than me :-).
I don't have a specific answer myself but I'd like to know your
thoughts.
Ok.....
I've spotted a pattern in a couple of our services, namely a
LoginService and a StudentService which are doing very similar things
with session management.
These methods are....
LoginService
- hasLogin()
- setCurrentLogin()
- getCurrentlLogin()
StudentService
- setCurrentStudent()
- getCurrentStudent()
The approach I wanted to take with all this was to centralise all
things "Login" and "Student" into single services. That way the Team
here don't have to worry about where they go depending on the
scenario.
The thing is I can now see we're going to need a few more services
along the same lines and I want to start thinking about refactoring
these session parts into a more central way of working. Almost a way
of giving those services that need it a session management mechanism.
I've 2 main issues:-
1) I don't know if simply inheriting from a session service is the
best approach. The way of working with inheritance I was always told
is to use an "is a" approach. My gut instinct is telling me that these
services are not an extension of the session services but more a "has
a" session management mechnism. Maybe this suits another Design
Pattern that would keep coupling looser. Maybe I'm wrong and tight
inheriting is the best way :-)
2) I need to make these methods more generic so more along the
approach of hasCurrentObject(), set / get CurrentObject() approach.
These will need to pass back specific types though depending on the
service i.e .the session Login, Student objects.
One idea I had with this was to use a property in the service to
specific the type but again I'm not sure if that is the best way.
I don't think passing an arguments to the methods is the best...
reason being that the calling handler is then aware of the internal
object being managed and if it knows that then why bother with the
service at all.
Look forward to your thought of wisdom on this one :-)
Cheers,
James
Jatin Nanda
--
You received this message because you are subscribed to the Google Groups "Object-Oriented Programming in ColdFusion" group.
To post to this group, send email to coldfu...@googlegroups.com.
To unsubscribe from this group, send email to coldfusionoo...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/coldfusionoo?hl=en.
Kevin Roche
Bear in mind the rule: "Favour composition over inheritance."
I would create a session manager service which can be used by any
object.
You can then inject session manager service into the objects.
Kevin Roche
Baz
James Buckingham
Jatin - Sounds interesting. I'll look into cfinterface, thanks :-). Is
interfaces maybe looked at in the same sense as inheritance though
when looking at coupling? Am I right in thinking that its a way of
making an object follow a contract in how it behaves?
---
Kevin - Of the same thinking as me mate :-). I'd rather not look into
inheritance but I'm not sure how I can do this through injection
either.
Just to give a bit more info these services are the main model parts
of our new ColdBox application. The services that are managing the
session side of things are really just facades to the ColdBox
SessionStorage plugin. I could just access them directly but as I say
in the original posting I wanted to give the Developer a single port
of call for all things Student, Login, User.... whatever". It just
simplified things for them and shrinks the code down in the Event
Handlers.
So I guess in terms of session manager and injection that's being done
by ColdBox.
---
Baz - The issue is more to do with "What Services are going to be
managing sessions and are going to need these commonly used methods",
rather than specifically what services are going to need to manage
which objects. I'm trying to separate things like Student, Teacher,
Member etc. objects into a one-to-one relationship with their
services. So I can't see a crossover, not at this point anyway.
Yeah I'm not looking to hide the CF session as such. I'm really trying
to teach them to look to the Services as a toolbox of all things they
would need to get the job done. It's just trying to push a more OO
approach and this seems to be a good way of demonstrating how it can
be done and as you say grouping the session calls to their own
"services" (files) would also make it easier to managing.
---
Gut instinct is telling me that I'm overly complicating things at this
stage and trying to make things reusable to the "Nth" degree when it
might not be needed. At the same time I don't want to repeat all these
methods every where - I don't think there is a right / wrong answer
which is bit frustrating :-)
Sean Corfield
> LoginService
> - hasLogin()
> - setCurrentLogin()
> - getCurrentlLogin()
>
> StudentService
> - setCurrentStudent()
> - getCurrentStudent()
> The thing is I can now see we're going to need a few more services
> along the same lines and I want to start thinking about refactoring
> these session parts into a more central way of working. Almost a way
> of giving those services that need it a session management mechanism.
"session" is merely an implementation detail and there's no guarantee
that current student and current login will always share the same
implementation.
> 1) I don't know if simply inheriting from a session service is the
> best approach. The way of working with inheritance I was always told
> is to use an "is a" approach.
Right. "session service" is, at best, an implementation detail and,
frankly, I'm strongly against scope-based facades since they actually
create global variables and stronger coupling (since all services that
use such scope-based facades now have access to the same *global* pool
of data and are all tied together by that data - whereas previously
they notionally only had access to their own data and were independent
of each other).
> 2) I need to make these methods more generic so more along the
> approach of hasCurrentObject(), set / get CurrentObject() approach.
Why?
You're conflating an implementation detail with your API which is a
bad thing. As noted above, using session scope is an implementation
detail and there's no guarantee you would always use session scope for
all such "similar" things.
Furthermore, wrapping a shared scope with a generic API just adds
overhead for no real benefit:
if facade.hasCurrentObject("name")
x = facade.getCurrentObject("name")
facade.setCurrentObject("name",value)
vs
if structKeyExists(scope,"name")
x = scope.name
scope.name = value
The latter is both easier to read and faster.
You *might* have a good argument for encapsulating the raw storage
mechanism as a private method within the service - assuming you want a
struct/scope-based implementation:
if structKeyExists(getCurrentStorage(),"name")
x = getCurrentStorage().name
getCurrentStorage().name = value
and
private function getCurrentStorage() { return session; }
Now you can modify just one method (in each service) to change how the
current thing is stored but I really don't think it buys you much (if
anything).
I think you're over-analyzing.
--
Sean A Corfield -- (904) 302-SEAN
Railo Technologies, Inc. -- http://getrailo.com/
An Architect's View -- http://corfield.org/
"If you're not annoying somebody, you're not really alive."
-- Margaret Atwood
James Buckingham
Get the feeling this is a touchy subject with you :-)
As it happens we ended up pulling the session management out the
service. The result was, as you say, we've now got a cleaner API for
the Login Service and it also meant we could move the returning
objects around in the handlers a lot easer.
It's also helped in defining our Unit Tests a lot better, so yeah all
good :-).
Thanks again,
James
Sean Corfield
> Get the feeling this is a touchy subject with you :-)
Yeah. I see scope facades repeatedly in CFML code and it's just plain
ol' wrong. Some frameworks even enshrine this nonsense in their API so
the framework itself is providing you with a completely unencapsulated
GLOBAL variable dressed up in the overhead of a set of method calls.
Dreadful!!
James Allen
variable accessors in the relevant service right? The classic
getCurrentUser(), setCurrentUser() etc..?
---
James Allen
E: ja...@jamesallen.name
Blog: http://jamesallen.name
Twitter: @CFJamesAllen (Coldfusion / Web development)
Twitter: @jamesallenuk (General)
Twitter: @JamesAllenVoice (Voiceover)
--
Sean Corfield
> Just to check Sean, you still favour the use of session / client (etc)
> variable accessors in the relevant service right? The classic
> getCurrentUser(), setCurrentUser() etc..?
Yes, preferably with the actual storage isolated to a single private method.
It's easy to test - either by extension and overriding or by method injection.
It isolates the variables - there's no global facade that provides
access to all session variables (or client variables or whatever).
It keeps variables independent in terms of storage - you can change
how an individual variable is actually stored, without worrying about
affecting other variables.
It encapsulates what is actually stored - this is important in the ORM
world where you need to be careful about detached objects (which you
get when you store _objects_ in session scope rather than just their
PKs). getCurrentUser() can / should encapsulate that: by relying on
the PK in session scope and fetching the object on demand per request
(Hibernate caches it anywhere per request - or, more accurately, per
Hibernate session).
The "SessionFacade" is a monstrous antipattern, IMO, particularly in
CFML. Unlike many languages, CFML provides long-lived scopes that span
multiple requests. Wrapping such global scopes in a bean doesn't
encapsulate anything (especially if it's actually called SessionFacade
- because then you still have "session" all over your app!). A scope
facade creates overhead on the global scope without adding any access
control - so any code that has the scope facade injected can access
*any* of the variables in that scope, just by calling methods. Other
languages that don't have these long-lived scopes have to create
abstractions, services and facades in order to provide an API that
does what CFML offers out of the box.
It's similar to all the nonsense I see about the Singleton pattern.
It's complicated in Java because Java has worked so hard to remove
global variables, but singletons are exactly that: global variables.
CFML provides a single, thread-safe point for initializing singletons
- onApplicationStart() - and a "global access" method, namely
application scope. CFML has near-zero-ceremony singletons built-in!
James Allen
I got this approach off you a few years ago and still find it an excellent
way to encapsulate anything that needs to persist over a users session. Just
'feels' right. I also have getCurrentStoredUser() for fetching a user who
has elected to have their details 'remembered'. Works a treat.
---
James Allen
E: ja...@jamesallen.name
Blog: http://jamesallen.name
Twitter: @CFJamesAllen (Coldfusion / Web development)
Twitter: @jamesallenuk (General)
Twitter: @JamesAllenVoice (Voiceover)
-----Original Message-----
From: coldfu...@googlegroups.com [mailto:coldfu...@googlegroups.com]
On Behalf Of Sean Corfield
Sent: 03 November 2010 19:59
To: coldfu...@googlegroups.com
Subject: Re: [coldfusionoo] Re: Refactoring - How would you approach this?
--