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.

Let's say we have a method setFoo that sets a flag in a Bar object e.g.

class Baz { 
    public void setFoo(String foo)(
       ...
    }
}

inside setFoo, the method is doing some expensive load of the Bar object from the database.

class Baz { 
    public void setFoo(String foo)(
       Bar bar = expensiveGetBar();
       ...
    }
}

To avoid the need to get the Bar object again, the developer changes the method to return it.

class Baz { 
    public Bar setFoo(String foo)(
       Bar bar = expensiveGetBar();
       ...
       return bar;
    }
}

Questions are:

  1. Is this discouraged?
  2. If yes, why and what is the alternative
  3. Should the solution be adding some sort of cache instead?
  4. How would you prefix the method? set? get? seget? geset?
share|improve this question
1  
If setFoo operates on Bars, shouldn't it be one of Bar's methods to begin with? –  Mat Feb 8 '13 at 6:42
    
I would cache that expensive Bar and add a getBar that returns the cache if possible. That leaves your setFoo sensible while still optimizing perf (and of course, only do this after measuring that the cost of expensivegetBar is really making a difference) –  Kate Gregory Feb 8 '13 at 12:37
    
The relationship between foo, bar, and baz is what defines if this is reasonable. If you see changing the foo on your baz as influencing bar it makes sense. If the relationship is an implementation detail then it doesn't. –  Sign Feb 8 '13 at 13:17
    
@Mat good point, but not always. e.g. Baz can be a service and Bar can be a Model loaded from the database, you can argue if this is good design or not (e.g. in Rails/Grails/Roo all CRUD is on the model, whereas in older Java/Spring typical architecture, you have a separate Dao / Service layer, it's a matter of taste). But I don't see how this relates to the question :) –  Eran Medan Feb 8 '13 at 14:43

4 Answers 4

up vote 9 down vote accepted

A function should serve one purpose. That’s part of the reason it’s called a function. If you find a method hard to name because it’s doing more than one thing, then split it into more than one method. Readable method names can often be made from just one verb and one noun.

In your example, you could change setFoo() to take a Bar explicitly. As a setter, it shouldn’t really need to return anything. Then either expose expensiveGetBar(), or expose getBar() and have it cache the result of expensiveGetBar().

share|improve this answer
1  
+1, your proposed changes are ok and may do the trick - in this case. But beware, when it comes to performance optimization, the "function should serve one purpose" rule does not always apply. –  Doc Brown Feb 8 '13 at 7:02
2  
@DocBrown: Fair enough. I’ve never actually come across an optimisation case where I had to break that particular rule of thumb. Usually optimisations involve changing data representations, eliminating indirections, caching, that sort of thing. You can kill a lot of extra abstraction before you have to start manually inlining functions! –  Jon Purdy Feb 8 '13 at 7:07
1  
+1 For adequately explaining the need to have one purpose per function. @DocBrown The one-purpouse rule is also very important to make your code readable and maintainable, especially if you may not end up being the only maintainer of the code for its lifespan. –  ddtpoison777 Feb 8 '13 at 8:08
    
Thanks, I like the one verb rule of thumb, the issue is that too often keeping rules of thumb gets harder than it may seem, the rules are meant to serve us, 90% of time they do, those 10% of time they don't is what troubles me, when figuring out the "right" way of doing it is not in line with "getting it done and shipping it". Not always clear what is better... I had developers that were much less productive because they had to do everything "right", on the other hand, others were productive but hard to maintain their code, the golden path in the middle is hard to find... –  Eran Medan Feb 8 '13 at 16:47

Here is my favorite solution for that problem:

class Baz { 
    Bar _bar;

    /* make this public if you like */ 
    Bar getBar()
    {
       if(_bar==null)
           _bar = expensiveGetBar();
       return _bar;
    }

    public void setFoo(String foo)
    {
       Bar bar = getBar();
       ...
    }
}

You may have to add a method to clear or update _bar, but that depends on your actual case.

share|improve this answer
    
Nice, pretty much what I had in mind, but there goes all the thread safety and synchronization aspect of it, also need to handle clearing the cached result etc... –  Eran Medan Feb 8 '13 at 14:46

Getters and Setters are a means to expose properties of classes. They are one-way functions. They either get (return a value) or they set (accept a value), but they don't do both in the same function. Even C# properties which do away the extra get and set functions maintains the one-way operation.

So if I have

  • GetFoo()
  • SetFoo()

Then there is an implied operation upon a property called Foo. Due to the magic of encapsulation, I don't really care if the internal object is called Foo. I just know that if I SetFoo() and then GetFoo() then there should be some degree of logical linkage between my operations.

So you don't have getters | setters in your example. Yes, I know they are named that way, but that's not what they're doing. The third snippet really shows the deviation from the concept of properties. public Bar setFoo(String foo) means I set a Foo but I get a Bar back? What?!

You need to pick a different verb than 'Set' to use in this case, because the function is not performing a set operation. Something like public Bar AnalyzeFoo(String foo) may be a more appropriate description of what the function is doing and still hides the fact that there is a really expensive operation on Bar as part of the function.

A more concrete example may help.

This next example is bad, for the reasons explained above:
public ProfitMargin SetInvoice(Invoice inv)

But this example makes more sense:
public ProfitMargin AnalyzeInvoice(Invoice inv)

I don't know or need to know how expensive of an operation it is to break apart the Invoice and identify the profit margin; the AnalyzeInvoice() function takes care of that for me.


A quick note on what I meant with C# properties...

Yes, I still have to define the get & set routines but they are expressed as part of the Property and don't require additional functions like Java used to require.

So in C#, I have:

private ____ xyz
public ____ MyProp {
   get { .... return xyz; }
   set { .... xyz = value; }
}

Whereas old-school Java required explicit functions:

private ____ xyz;
public ____ GetMyPrep() { ... return xyz; }
public void SetMyPrep(____ value) { ... xyz = value; }

Both work well, and modern IDEs simplify the getter | setter creation. The point is that both approaches are still one-way operations.

share|improve this answer

I would think you might do the following:

    bar = getExpensiveBar();
    setBarState(bar);

    setBarState( Bar myBar)
    {
       myBar.setFlag(true);
       myBar.setOtherFlag(false);
    }

But If the Bar object just has a flag then wouldn't you just do the following:

    bar = getExpensiveBar();
    bar.setFlag(true);
share|improve this answer

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.