views:

237

answers:

1

Yes, I know, yet another question about mutable objects. See this for general background and this for the closest analogue to my question. (though it has some C++ specific overtones that don't apply here)

Let's assume that the following pseudo code represents the best interface design. That is, it's the clearest expression of the business semantics (as they stand today) into OO type. Naturally, the UglyData and the things we're tasked to do with it are subject to incremental change.

public class FriendlyWrapper
{
    public FriendlyWrapper(UglyDatum u)
    {
        Foo = u.asdf[0].f[0].o.o;
        Bar = u.barbarbar.ToDooDad();
        Baz = u.uglyNameForBaz;
        // etc
    }

    public Widget Foo { get; private set; }
    public DooDad Bar { get; private set; }
    public DooDad Baz { get; private set; }
    // etc
    public WhizBang Expensive1 { get; private set; }
    public WhizBang Expensive2 { get; private set; }

    public void Calculate()
    {
        Expensive1 = Calc(Foo, Bar);
        Expensive2 = Calc(Foo, Baz);
    }

    private WhizBang Calc(Widget a, DooDad b) { /* stuff */ }

    public override void ToString()
    {
        return string.Format("{0}{1}{2}{3}{4}", Foo, Bar, Baz, Expensive1 ?? "", Expensive2 ?? "");                             
    }
}

// Consumer 1 is happy to work with just the basic wrapped properties
public string Summarize()
{
    var myStuff = from u in data
                  where IsWhatIWant(u)
                  select new FriendlyWrapper(u);

    var sb = new StringBuilder();
    foreach (var s in myStuff)
    {
        sb.AppendLine(s.ToString());
    }
    return sb.ToString();
}

// Consumer 2's job is to take the performance hit up front.  His callers might do things 
// with expensive properties (eg bind one to a UI element) that should not take noticeable time. 
public IEnumerable<FriendlyWrapper> FetchAllData(Predicate<UglyDatum> pred)
{
    var myStuff = from u in data
                  where pred(u)
                  select new FriendlyWrapper(u);

    foreach (var s in myStuff)
    {
        s.Calculate();  // as written, this doesn't do what you intend...
    }

    return myStuff;
}

What's the best route here? Options I can see:

  1. Mutable object with an explicit Calculate() method, as above
  2. Mutable object where expensive calculations are done in the getters (and probably cached)
  3. Split into two objects where one inherits (or perhaps composes?) from the other
  4. Some sort of static + locking mechanism, as in the C++ question linked above

I'm leaning toward #2 myself. But every route has potential pitfalls.

If you choose #1 or #2, then how would you implement Consumer2's loop over mutables in a clear, correct manner?

If you choose #1 or #3, how would you handle future situations where you only want to calculate some properties but not others? Willing to create N helper methods / derived classes?

If you choose #4, I think you're crazy, but feel free to explain

+1  A: 

In your case, since you're using LINQ, you're only going to constructing these objects in cases where you want the calculation.

If that is your standard usage pattern, I would just put the expensive calculation directly in the constructor. Using lazy initialization is always slower unless you plan to have some cases where you do not calculate. Doing the calculation in the getters will not save anything (at least in this specific case).

As for mutability - mutable objects with reference syntax and identity (ie: classes in C#) are really okay - it's more a problem when you're dealing with mutable value types (ie: structs). There are many, many mutable classes in the .NET BCL - and they don't cause issues. The problem is typically more of one when you start dealing with value types. Mutable value types lead to very unexpected behavior.

In general, I'd turn this question upside down - How and where are you going to use this object? How can you make this object the most performant (if it's been determined to be problematic) without affecting usability? Your 1), 3) and 4) options would all make usability suffer, so I'd avoid them. In this case, doing 2) won't help. I'd just put it in the constructor, so your object's always in a valid state (which is very good for usability and maintainability).

Reed Copsey
Sounds good in general. In the code that prompted me to write the question, though, 90% of instances are constructed in places like Consumer 1 where I'm simply wrapping away the ugliness. Only 10% of the time do I do any calculations. In addition, one of my Calc() operations recursively queries for more UglyData. So your solution means I can't use my friendly wrapper there at all, or I'd have an infinite loop.
Richard Berg
Went ahead and marked this as the answer. I actually used option #3 in my app. But my motivation was Reed's principle idea that you should always be valid once constructed. Thus, my newly stripped down FriendlyWrapper and the FriendlyWrapperWithExpensiveStuff class that inherits from it both do everything they need in their constructors. To enhance usability, the Expensive class has an overloaded constructor that takes in any existing Friendly class.
Richard Berg