Should nested classes be used to define simple return types for data access?

Question

Say I have a class to get various pieces of info about sports scores, teams, etc:

public class SportsInfo
{
    GameResult GetResult(string date, string homeTeam){...}
    TeamRecord GetRecord(string teamName){...}

    public class GameResult
    {
        public int HomeTeamScore { get; set; }
        public int AwayTeamScore { get; set; }
    }

    public class TeamRecord
    {
        public int Wins { get; set; }
        public int Losses{ get; set; }
    }
}

It seems most advice I see online is that nested classes should generally be private, and only used for types that only make sense inside the containing class. However, in this case, the return types are extremely small and essentially just glorified key-value pairs. The other option is to break these out to be non-nested classes, but in many environments it is expected that you have only 1 class per file, meaning I would have an entire file, including header and other overhead just to define a class that has just 2 auto-properties. To me, that makes navigating, building, and even understanding the code more difficult vs having the classes easily visible, nested inside the parent SportsInfo class.

Is there any accepted best practice on this? Obviously if the number or size of return types increases enough, it would interfere with the clarity of SportsInfo.cs, and should then be broken out. But provided the records are this simple, isn't it easier and clearer as written above?

Answer 1

Nested classes as data holders make sense to me. However, you need to consider several other factors.

1. The example shows only scalar contents, in which case would it be better to use a struct to make it value-like?
2. In these classes you have shown setters, but wouldn't it be better if they were immutable?
3. If there were reference value fields and they are not immutable, wouldn't you need a copy constructor?
4. If A calls B and B returns a value and A expects that value then there is a contract between them, but in a nested class the contract is rather one-sided.
5. How would you reconcile this strategy with defining contracts in terms of interfaces rather than concrete classes?

I guess what I'm saying is that in the (relatively) narrow situation of scalar fields and/or immutable values with concrete implementations it looks OK. I suspect that as you break out of that niche they will cause more trouble than they are worth. YMMV.

Answer 2

Framework design guidelines says that

• AVOID publicly exposed nested types. The only exception to this is if variables of the nested type need to be declared only in rare scenarios such as subclassing or other advanced customization scenarios.
• DO NOT use nested types if the type is likely to be referenced outside of the containing type.

So, say no to public nested types.

Answer 3

To add more to the previous answer that mentioned MS design guidelines:

Purpose of nested type is to access it's enclosing type private details as part of implementation and target complex types implementation scenarios, such as internal state, highly coupled collections implementing other public interfaces or subclassing public types, etc.

in your case GameResult and TeamRecord are data objects and thus don't benefit from being nested, at the same time consumer of your class will have less readable and maintainable code. So use of nested classes here is unnecessary and counterproductive.