Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

So I have a webservice that has something like a getAccount where it would return an identifier to the account if it got it, else throw an exception. The client will always want to create an account if an exception is thrown with the same info the get is done with.

I am creating a convenience library for clients that will be handling all of the webservice calls inside so they don't need to know how to do the calls themselves.

What I am wondering is in this library if I were to create a getAccount(accountName) that will get the account if it exists, and if it does not then create it and return the info, is that a bad thing to do? Should I leave it to the client to handle the exceptions or simply name it something like getOrCreateAccount? Does it matter?

Is it bad practice to create something in a get operation?

share|improve this question
6  
At the very least, I would name it getOrCreateAccount or similar. –  Telastyn May 22 '12 at 19:08
4  
It seems valid in the context of lazy initialization. However, it depends if you have a need to use that pattern in your library. –  Chris C May 22 '12 at 19:43

5 Answers 5

up vote 16 down vote accepted

No, it is not 'bad practice'. So long as you and the other developers agree thats how you want it to work, its just fine. After all, it'd be returning an account, which is what you want. That the account is created 'under the hood' is irrelevant to the caller.

share|improve this answer
6  
Only exception is if it's a publicly available API. The larger community has already agreed that GET is indempotent and nullipotent; in other words, the same action is performed each time, and it's a safe method that doesn't change the state of the server. This is in the REST Wiki. Aside from that, if the API just exists in your own little world, do whatever is accepted by your group. –  jmort253 May 22 '12 at 20:11
9  
I disagree, even for a public API its fine - so long as it makes sense within the context of whatever the API is supposed to do. There's little point in creating multiple entries in the API when a single one would do the trick. –  GrandmasterB May 22 '12 at 20:21
    
For the record it is an entirely internal library and I have the ability to just tell the team using it how it works –  Mike May 22 '12 at 20:35
    
@jmort253: I think the service is simply named getAccount, it doesn't necessarily use an HTTP GET. TBH, that was my first thought, too. –  TMN May 22 '12 at 22:34
    
@GrandmasterB - Sorry, I was thinking REST, as in HTTP API's like Google's Weather API's, etc, etc. and all that. For a code library, I agree 100% with your points. Use whatever makes the most sense. +1 –  jmort253 May 22 '12 at 23:53

It depends on the circumstances.

For example, you can use it to do lazy loading/instantiation, deferring the loading of data or the creation of an instance until it's actually needed. This is normally sensible as it saves resources that you might not need (if the class/data is never needed then it's never loaded).

However, in this particular case, I'd say that having a method called getAccount that would create a new account if it didn't exist would not be good practice. If the user has given some credentials to identify a particular account and that account couldn't be found, does that mean that the user is not yet a customer and should have an account created for them, or does it mean that the credentials were mistyped and the user needs to be asked to verify that they've entered what they meant to enter?

If you have a getAccount method that does create a new account if it couldn't identify one, then you have no choice in the matter. If you split account creation and account getting out into separate methods, then you have a lot more flexibility in deciding what to do should an attempt to get an account fails.

share|improve this answer

It makes most sense to create 3 methods:

getAccount -> Which just gets the account.

createAccount -> Creates an account.

getAccountAndCreateIfNeeded -> Choose your own naming ;)

Why separation: You have a simple method for getting and creating. That is a clear testable method for both. For getAccount it is not an exception to not find the account. So just return false or something like that, it is expected.

Then you can use that return value in your grouped function: getAccountAndCreateIfNeeded which is now also testable, it should return an account always. Whatever you ask.

All these 3 methods are clear, it is exactly clear what they do and what they return. You may make agreements now with your team but this kind of exceptions are terrible on the long run. Just make them very clear and you will have no issues.

share|improve this answer
    
Also, from Clean Code: "If your method can't do what its name implies, then throw an Exception." I'd have a Try/Catch block inside 'GetOrCreateIfneeded' that throws on a failed GET and then has the 'createAccount' inside the Throw for whatever exception type comes back for the failed get. –  Graham Aug 1 '12 at 19:21

If getAccount() can always return an account, then from the caller's point of view, the account does exist, and has always existed. There is no need to for getAccount() to 'create' anything. The account doesn't have to be stored anywhere until it is different from the default account.

share|improve this answer
    
+1 Generally, GetOrCreate is the wrong semantic but getting an object which may "logically" exist whether or not it phyiscally exists is fine. As an example, a sparse array of mutable items may not have any storage allocated for element 1,841,533, but still allow that element to be "retrieved" by creating a new object, storing it, and returning a reference. –  supercat Aug 26 at 21:39

Yes, it matters. In my opinion, it's a generally a bad practice to create something in a procedure that is not documented as having the powers of creation. Either name the procedure getOrCreate... or have a separate create... procedure and then if you really want, have a getOrCreate... that first attempts get..., and if that fails, calls create... and then calls get....

The user of the library will probably not expect the get... procedure to create if the get operation fails. If they suddenly find out that their test calls to get... created a whole tonne of data, they will probably be rather surprised. And how do they clean it up? What if they write code thinking that they will get an error if get... fails and they want to handle that their way?

share|improve this answer
    
Thanks, the create actually returns the id that the get would return too so I wouldn't have to do a get... create... get... just the first two. I will talk to the client about whether they will ever require the ability to simply call a get without wanting to create –  Mike May 22 '12 at 19:25
6  
@Mike: I still think it should have create in the name, just to be 100% clear about what's happening. –  FrustratedWithFormsDesigner May 22 '12 at 19:26
    
Yes I agree, I was planning on making it something along the lines of getOrCreate because my original thought was just to do get but I realized that it is not immediately clear that it is also creating something in the background –  Mike May 22 '12 at 19:31

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.