views:

212

answers:

6
public class BigPerformance  
{  
    public decimal Value { get; set; }
}  

public class Performance  
{  
    public BigPerformance BigPerf { get; set; }
}  

public class Category    
{  
    public Performance Perf { get; set; }     
}

If I call:

Category cat = new Category();  
cat.Perf.BigPerf.Value = 1.0;  

I assume this this breaks the Law of Demeter / Principle of Least Knowledge?
If so, how do I remedy this if I have a large number of inner class properties?

A: 

This is not breaking the Law of Demeter, because you are using public contract of classes.

Aen Sidhe
This is incorrect; see Chris S's answer http://stackoverflow.com/questions/2666930/how-to-modify-code-so-that-it-adheres-to-the-law-of-demeter/2667019#2667019
Spoike
Getter of the property is a method.
Aen Sidhe
A: 

I'm pretty sure your code will not work because Perf would be null. I'd do this

Category cat = new Category()
{
    Perf  = new Performance()
    {
        BigPerf = new BigPerformance()
        {
            Value = 1.0;
        }
    }
}
juharr
+2  A: 

If you're talking about Law of Demeter as in, "don't call the neighbors of neighbors" you can delegate it to other methods that do what you want.

From your example I'm guessing you want to reset the performance value or something. You can modify the example code so they are chained inherently instead:

Category cat = new Category();

cat.resetPerf();

The code would be something akin to this:

public class BigPerformance 
{
    //constructors 'n stuff

    public static decimal DEFAULT;

    public decimal Value {get; private set;}

    public void reset() {
        Value = BigPerformance.DEFAULT;
    }
}

public class Performance
{
    //constructors 'n stuff

    private BigPerformance BigPerf {get; set};

    public reset() {
        BigPerf.reset();
    }
}

public class Category
{
    // constructors 'n stuff

    public Performance Perf {get; private set;}

    public resetPerformance() {
        Perf.reset();
    }
}

That way the Category class doesn't need to know how to reset the value in case the default value is something different or it's type is changed in the future.

Personally if the risk for change is low I'd go for juharr's answer instead.

Spoike
+1  A: 
Category cat = new Category();  
cat.Perf.BigPerf.Value = 1.0;  

is

Category cat = new Category();  
cat.GetPerf().GetBigPerf().SetValue(1.0);  

So it is breaking the rules if the wikipedia definition is correct:

..[M]ethod M of an object O may only invoke the methods of the following kinds of objects:

  • O itself
  • M's parameters
  • any objects created/instantiated within M
  • O's direct component objects
  • a global variable, accessible by O, in the scope of M

In particular, an object should avoid invoking methods of a member object returned by another method

If you are worried about the 3 being tightly coupled, then remove the public accessors and add a method on Category to set the value. Then refactor Performance and BigPerformance to be private members.

Chris S
+7  A: 

One of my favourite quotes from Martin Fowler:

I'd prefer it to be called the Occasionally Useful Suggestion of Demeter

http://haacked.com/archive/2009/07/14/law-of-demeter-dot-counting.aspx

Rob Fonseca-Ensor
+1 Exactly what I was going to say. It's not a "dot counting task".
Finglas
A: 

If you always keep testabillity of your classes in mind and use IoC, you will notice you don't have to worry about LoD as much.

Look at it this way

How am I going to test Category? I don't want it to automagically create a Performance that's using the slow filesystem. Let's pass a IPerformance through to Category and replace the actual implementation with a dummy Performance instance.

How am I going to test Performance? I don't want it to automagically create a BigPerformance making a connection to a database. Let's pass a IBigPerformance through to Performance and replace the actual implementation with a dummy BigPerformance instance.
...
you obviously notice the pattern

Your code would be in the line of

BigPerformance BigPerf = new BigPerformance();
BigPerf.Value := 1.0;
Performance Perf = new Performance(BigPerformance);
Category cat = new Category(Performance);

(This would be retrieved from a factory.)

It looks (and in the shortrun probably is) like a lot more work but the benefits will pay off in the long run by being able to test your classes in isolation.

Take a look at Misco Hevery's blog for an eye opener on this and other subjects.

Lieven