Help Us Name This Method
Posted by Davy Brion on April 24th, 2009
This worked pretty good last time so i’m gonna try it again. We have a Repository base class (yes we still use it, and no, i’m not going to get into the discussion that’s going on about in on other blogs) which uses NHibernate underneath. We have the typical Get method which retrieves an object based on an ID. This method uses NHibernate’s ISession’s Get method, which either returns the entity if it’s present in the database, or it returns null.
Some people want to put an equivalent to NHibernate’s ISession’s Load method into the base repository class, but we just can’t come up with a good name for it. In case you don’t know, ISession.Load either returns the actual entity if it’s already in the session cache, or it returns a proxy for the entity but it does not immediately hit the database. The functionality of the Load method is thus very different from the Get method. Now, i think this is a case of extremely bad naming within NHibernate, and i don’t want to have the same naming issue in our Repository implementation.
So basically, the question is: how would you name the equivalent of the Load method? Remember: this method returns a proxy to an entity if it’s not yet in the session cache, whereas the Get method actually retrieves the entity from the database immediately or returns null if the entity for the given ID does not exist.
April 24th, 2009 at 3:53 pm
Assuming you treat the returned object as a proxy, as you are not sure (without checking) whether you got a real object or not. So you could name the method GetProxy.
April 24th, 2009 at 3:56 pm
But should you treat it differently because it is a proxy, or not? After all, as far as your code is concerned, you get an instance of YourEntity and your code should work regardless of whether it is a proxy or not.
April 24th, 2009 at 4:50 pm
The problem with this is that it breaks rather badly your persistance ignorance…
In which case do you chose the Get method or the Load one ?
April 24th, 2009 at 6:01 pm
I’d think this would be an easy one for you Davy, considering your recent efforts in this area:
FutureGet
April 24th, 2009 at 6:11 pm
Interesting dilema.
Not sure all the usages scenarios in your case but a few quick ideas (without much thought behind theme).
_repo.GetFromCache(id);
Using “from cache” indicates that you may not get the actual object and doesn’t necessarily breaks persistance ignorance) an alternatived name could be: _repo.GetCached(id).
Now, if you use another cache for other purposes like memcached this method name may be misleading.
_repo.TryToGet(id);
Again this is not perfect, but indicates that you may not actually get the instance.
A more verbose (but precise) naming approach.
_repo.GetInstanceOrProxy(id); //I may personaly get this one.
April 24th, 2009 at 6:13 pm
I’d go for GetProxyByID(….)
April 24th, 2009 at 6:13 pm
Or
GetObjectReferenceByID
GetObjectLocatorByID
April 24th, 2009 at 6:21 pm
LazyGet or
FutureGet (as Bill proposed)
April 24th, 2009 at 7:07 pm
I like .Imagine()
If you already have what you imagine, then you have it. If you don’t already have what you imagine, you get to play with a make-believe one.
April 24th, 2009 at 9:22 pm
GetInstanceOrProxy(id) makes sense and does what it says it does.
April 24th, 2009 at 9:45 pm
[...] Help Us Name This Method – Davy Brion Davy has trouble making up his own names method, so could you please help him out? [...]
April 24th, 2009 at 11:24 pm
Isn’t it a viable option to provide an overload of the Getmethod with an extra boolean to specify if you want to immdiate DBaccess or not?
April 25th, 2009 at 12:37 am
I’d go for GetCachedOrLazy(id). ‘Instance’ doesn’t really imply not touching the database, and ‘Proxy’ is just an implementation detail I think.
April 25th, 2009 at 10:55 am
Hi Davy.
I have the exact same problem at hand now. We’re setting up the Persistence layer, using NH, and looking for the correct naming.
My (not final) take is to map NH.Load to repo.Get, and NH.Get to repo.FindById
reasoning:
The NH.Load method is supposed to be used when you know that the ID is present in the data store. Otherwise, an exception will be thrown. the .Get behaviour adhere to search mechanism – that is: if it’s there then bring it back, otherwise return null. It behaves exactly like “from Whadever o where o.Id == Id”.List().FirstOrDefault() + first look in Session cache.
April 25th, 2009 at 1:16 pm
FutureGet: at first i liked this one, but i think it would be too easy to think that the actual query (if it’s ever sent) would be batched along with other queries if possible (which is the behavior of every other FutureSomething method name). But the ISession.Load method won’t do any batching.
GetInstanceOrProxy: might be a good one, though i’d probably go for GetProxyOrInstance then, since it’s often more likely that you’ll end up with a proxy than with the actual instance
@Rob: public methods with a boolean parameter that modify the behavior of the method are a bit of a no-no. It’s far better to come up with a meaningful name which clearly explains the different behavior… not always easy if you can’t come up with a good name though
@Ken: i kinda like the reasoning behind that, but unfortunately we’ve already had the Get method in the base class for months now and it’s already being used all over the place, so i can’t just change it like that.
another possibility i came up with is DeferredGet… the name is not entirely accurate though since the actual retrieval may never happen… thoughts?
April 25th, 2009 at 4:17 pm
in uNhAddIns application blocks the name is GetProxi, in another app I’m using GetDelayed
April 25th, 2009 at 6:37 pm
@Davy – resharper can help with the rename, and smtp/pop3 with communicating that to the team
I’m gonna go with that for our team, so hopefully soon enough I’ll have input from people who have never used NH about this naming convention.
April 26th, 2009 at 4:17 pm
Why not create an enumeration
public enum LoadingBehaviour {
///<summary>Load from the data store immediately,
///raising an exception if the item is not found</summary>
Force,
///<summary>Return the cached object if it exists, otherwise
///create and return a proxy that may later raise an exception
///when it's used</summary>
Defer
}
and then add an overload like .Get(int id, LoadingBehaviour behaviour), with the default .Get(int id) calling the new overload with the appropriate default loading behaviour?
It means when you’re reading the code, you’ll see
customerRepository.Get(customerId, LoadingBehaviour.Defer)
- which indicates pretty clearly what’s going on, without polluting your repository interface with potentially confusing method names.
Just an idea
-D-
April 27th, 2009 at 4:32 pm
GetProxiableInstance
April 27th, 2009 at 11:40 pm
Should the calling code actually care? Or is it more a strategy configured through you ioc?
April 28th, 2009 at 7:51 am
@Rob
There are cases where you specifically want to use proxies for performance reasons, so yes, the calling code should care
The difference in behavior is not something i’d want to configure through an IOC container, since the choice of using a proxy instead of fetching the real reference should in most cases be an explicit one IMO
As for where we stand on the name of the method… we’re currently leaning towards the overloaded Get method with the enum as mentioned by Dylan. It seems to be the only option that nobody objects to
April 29th, 2009 at 10:29 pm
I too vote for either LazyGet or FutureGet
July 6th, 2009 at 1:13 pm
Cheers for this. I had the same problem
I went for the LoadingBehaviour option as well. It’s an unnecessary overload in some respects but it is obvious what its for.