views:

319

answers:

8

I have a class that I have to call one or two methods a lot of times after each other. The methods currently return void. I was thinking, would it be better to have it return this, so that the methods could be nested? or is that considerd very very very bad? or if bad, would it be better if it returned a new object of the same type? Or what do you think? As an example I have created three versions of an adder class:

// Regular
class Adder
{
    public Adder() { Number = 0; }

    public int Number { get; private set; }

    public void Add(int i) { Number += i; }
    public void Remove(int i) { Number -= i; }
}

// Returning this
class Adder
{
    public Adder() { Number = 0; }

    public int Number { get; private set; }

    public Adder Add(int i) { Number += i; return this; }
    public Adder Remove(int i) { Number -= i; return this; }
}

// Returning new
class Adder
{
    public Adder() : this(0) { }
    private Adder(int i) { Number = i; }

    public int Number { get; private set; }

    public Adder Add(int i) { return new Adder(Number + i); }
    public Adder Remove(int i) { return new Adder(Number - i); }
}

The first one can be used this way:

    var a = new Adder();
    a.Add(4);
    a.Remove(1);
    a.Add(7);
    a.Remove(3);

The other two can be used this way:

    var a = new Adder()
        .Add(4)
        .Remove(1)
        .Add(7)
        .Remove(3);

Where the only difference is that a in the first case is the new Adder() while in the latter it is the result of the last method.

The first I find that quickly become... annoying to write over and over again. So I would like to use one of the other versions.

The third works kind of like many other methods, like many String methods and IEnumerable extension methods. I guess that has its positive side in that you can do things like var a = new Adder(); var b = a.Add(5); and then have one that was 0 and one that was 5. But at the same time, isn't it a bit expensive to create new objects all the time? And when will the first object die? When the first method returns kind of? Or?

Anyways, I like the one that returns this and think I will use that, but I am very curious to know what others think about this case. And what is considered best practice.

A: 

Consider this: if you come back to this code in 5 years, is this going to make sense to you? If so, then I suppose you can go ahead.

For this specific example, though, it would seem that overloading the + and - operators would make things clearer and accomplish the same thing.

Adam Robinson
Well, of course :p In this case the best would have been to use an int directly kind of. But it was just what I could come up with as a simple example of nested methods. My focus was the method calling, not what they actually did :p
Svish
Except when you come back in 5 years. A method chain is going to reveal itself instantly with one mouse hover while operators require looking at the class itself.
Samuel
ah, that is a good point yes.
Svish
+8  A: 

The 'return this' style is sometimes called a fluent interface and is a common practice.

Brian
aha. cool. then I guess I will go for that then =)
Svish
Note that the fluent style writes similarly to a very different alternative, which is to make your object immutable and return new instances of your object instead of modifying internal state. With a simple class like the one in the example (not that the example is very realistic) you might want to consider the pros and cons of simply making it immutable instead.
mquander
Both `return this` and `return new` are examples with different applications: jQuery `$('#eleId').show()` returns `this` because otherwise it would create a copy of eleId. Linq `myCollection.Where(x=>x.IsValid)` returns a new object because otherwise it would have changed myCollection, which could be immutable.
Keith
+3  A: 

I like "fluent syntax" and would take the second one. After all, you could still use it as the first, for people who feel uncomfortable with fluent syntax.

another idea to make an interface like the adders one easier to use:

public Adder Add(params int[] i) { /* ... */ }
public Adder Remove(params int[] i) { /* ... */ }

Adder adder = new Adder()
  .Add(1, 2, 3)
  .Remove(3, 4);

I always try to make short and easy-to-read interfaces, but many people like to write the code as complicated as possible.

Stefan Steinegger
Cool idea with the params usage. Might use that myself....
Svish
A: 
  1. For your specific case, overloading the arithmetic operators would be probably the best solution.

  2. Returning this (Fluent interface) is common practice to create expressions - unit testing and mocking frameworks use this a lot. Fluent Hibernate is another example.

  3. Returning a new instance might be a good choice, too. It allows you to make your class immutable - in general a good thing and very handy in the case of multithreading. But think about the object creation overhead if immutability is of no use for you.

Daniel Brückner
Nicely summed up =)
Svish
A: 

If you call it Adder, I'd go with returning this. However, it's kind of strange for an Adder class to contain an answer.

You might consider making it something like MyNumber and create an Add()-method.

Ideally (IMHO), that would not change the number that is stored inside your instance, but create a new instance with the new value, which you return:

class MyNumber
{
    ...

    MyNumber Add( int i )
    {
        return new MyNumber( this.Value + i );
    }
}
Lennaert
Well, of course. And even stranger to contain a method called Remove, which actually should have been called Subtract. But that wasn't the point here :p
Svish
A: 

I think that for simple interfaces, the "fluent" interface is very useful, particularly because it is very simple to implement. The value of the fluent interface is that it eliminates a lot of the extraneous fluff that gets in the way of understanding. Developing such an interface can take a lot of time, especially when the interface starts to be involved. You should worry about how the usage of the interface "reads"; In my mind, the most compelling use for such an interface is how it communicates the intent of the programmer, not the amount of characters that it saves.

To answer your specific question, I like the "return this" style. My typical use of the fluent interface is to define a set of options. That is, I create an instance of the class and then use the fluent methods on the instance to define the desired behavior of the object. If I have a yes/no option (say for logging), I try not to have a "setLogging(bool state)" method but rather two methods "WithLogging" and "WithoutLogging". This is somewhat more work but the clarity of the final result is very useful.

JonStonecash
+1  A: 

Chaining is a nice thing to have and is core in some frameworks (for instance Linq extensions and jQuery both use it heavily).

Whether you create a new object or return this depends on how you expect your initial object to behave:

var a = new Adder();

var b = a.Add(4)
         .Remove(1)
         .Add(7)
         .Remove(3);

//now - should a==b ?

Chaining in jQuery will have changed your original object - it has returned this. That's expected behaviour - do do otherwise would basically clone UI elements.

Chaining in Linq will have left your original collection unchanged. That too is expected behaviour - each chained function is a filter or transformation, and the original collection is often immutable.

Which pattern better suits what you're doing?

Keith
Good observations about the thinking behind Linq and jQuery =) On the matter of "should a==b", in this exact silly case, they should of course be considered equal since they have the same number. But depending on if you used method 2 or 3, the references wouldnt really need to be the same.
Svish
If you use `new` each time after the operations complete a.Number == 1 while b.Number == 7. If you use `this` instead afterwards a.Number == 7 too.
Keith
A: 

The main difference between the second and third solution is that by returning a new instance instead of this you are able to "catch" the object in a certain state and continue from that.

var a = new Adder() .Add(4);

var b = a.Remove(1);

var c = a.Add(7) .Remove(3);

In this case both b and c have the state captured in a as a starting point. I came across this idiom while reading about a pattern for building test domain objects in Growing Object-Oriented Software, Guided by Tests by Steve Freeman; Nat Pryce.

On your question regarding the lifetime of your instances: I would exspect them to be elligible for garbage collection as soon as the invocation of Remove or Add are returning.