4

We have a large project that performs a lot of modelling and calculations. This code is being broken down into smaller, more manageable chunks, by shifting particular calculations into their own classes.

To try and keep the inputs and outputs constrained to the appropriate calculation method, two public nested property classes have been used (if property classes is the correct term for a class that only has properties).

The following is a made up example of how this is being approached:

public class SomeCalculation
{
    public Result Calculate(Parameters p)
    {
        double foundValue = 0;

        switch (p.ParamD)
        {
            case SomeEnum.Element1:
                foundValue = p.ParamA + p.ParamB + p.ParamC;
                break;
            case SomeEnum.Element1:
                foundValue = p.ParamA + p.ParamB - p.ParamC;
                p.ParamC = 5;                    
                break;
            default:
                p.ParamB = -2;
                break;
        }

        return new Result()
        {
            FoundValue = foundValue,
            NewParamB = p.ParamB,
            NewParamC = p.ParamC
        };
    }

    public class Parameters
    {
        public double ParamA { get; set; }
        public double ParamB { get; set; }
        public int ParamC { get; set; }
        public SomeEnum ParamD { get; set; }
    }

    public class Result
    {
        public double FoundValue { get; set; }
        public double NewParamB { get; set; }
        public int NewParamC { get; set; }
    }

}

This calculation would then be performed by instantiating and passing in the nested Parameters class. The main advantages of this is that the Parameters and Result classes are kept with the Calculation class, and the same nested classes names can be reused with other calculation classes.

I'm wondering if the above design pattern is a reasonable approach, even though the MSDN Nested Types page says to avoid publicly exposing nested types. Also, does it conform with standard practices (such as the SOLID principles) or will it have any limitations that could cause problems?

1
  • 1
    "and the same nested classes names can be reused with other calculation classes". In that case, call the classes SomeCalculationParameters and SomeCalculationResult. No need to nest the classes then.
    – David Arno
    Commented Oct 10, 2016 at 8:17

3 Answers 3

3

The reason that MSDN says to avoid publicly-exposed nested types is because, if you needed them public, then why would you also need the encapsulation that nesting provides?

Nested classes are therefore usually used in places where you would only use the class inside the outer class, and nowhere else. I've found occasion to do this exactly once. I don't like classic Tuples in C#, and this particular class included a small DTO that was not exposed to the outside world.

You're not going to need nested classes about 99 percent of the time. Generally, if it's important enough to make a class, it's important enough to make it public and not nested. Namespaces are a better place to put such classes.

Accessing a nested class from a public member is almost certainly a semi-violation of the "Law" of Demeter (even if you're just "counting dots").

2

Public nested types are generally just a cosmetic problem. There is functionally no real difference between nested and non-nested type other than their name. So it all depends on what you and your team feels. It would make a difference if nested type was private, but that is not your case.

But, I'm seeing another problem. Your method takes ParamB and ParamC and returns NewParamB and NewParamC. This is red flag to me, because it seems that you are actually separating something that should be single class, because multiple methods share some common state they manipulate. Blindly separating each class method into pure static method is going to make your code more complex if those methods actually belong to single class. You should put much more thought into creating class with methods that share and manipulate single state. Maybe you will find out there is more to your code than bunch of calculations.

4
  • "Public nested types are generally just a cosmetic problem". No, it's a coupling problem.
    – David Arno
    Commented Oct 10, 2016 at 8:14
  • @DavidArno If he is going to use the method, then his is already coupled to the class. So there is really no difference.
    – Euphoric
    Commented Oct 10, 2016 at 9:04
  • Sure, in the OP's case he's just defining POCOs, so the coupling (whether through nesting, or just reference) isn't an issue and the OP is using nesting as a form of namespacing. However, I disagree with your general assertion that "Public nested types are generally just a cosmetic problem"
    – David Arno
    Commented Oct 10, 2016 at 13:48
  • Seems that the preference is to separate the classes, and control their relationship by namespaces. Your second comment though is the one that has made me think more about how these classes operate, and that I should reevaluate their structure.
    – Ayb4btu
    Commented Oct 10, 2016 at 21:37
1

I might be wrong in this assertion, but this seems to me like you're trying to avoid the Single Responsibility Principle by just breaking up the functionality into separate classes and then putting those class definitions inside of another class. That, or you're attempting to separate functionality into separate classes when in reality it all relies on one another to function properly, in which case it should all be in a single class anyways. Either way I do not see why you are using nested classes here.

What you could try doing is making an interface that holds the different functions needed to fully make a calculation, and then just have each class implement that interface differently based on what type of calculation is being made. This has the added benefit of making your code polymorphic, which your current code doesn't appear to be. You also shouldn't need any other classes verifying that your input and output are valid. If you designed the class properly, it should do that on its own.

Or, if the implementation is going to remain mostly the same, with a few differences, you can also use an abstract class to do most of the implementation in a base class, and implement the rest in the sub classes.

Essentially what I'm saying is, keep your code modular. Don't split up implementation into a bunch of different classes, regardless of whether they are nested or not. A class should be able to function fully on its own, and if it does not, then it shouldn't be a class.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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