문제

(Refer Update #1 for a concise version of the question.)

We have an (abstract) class named Games that has subclasses, say BasketBall and Hockey (and probably many more to come later).

Another class GameSchedule, must contain a collection GamesCollection of various Games objects. The issue is that we would, at times, like to iterate only through the BasketBall objects of GamesCollection and call functions that are specific to it (and not mentioned in the Games class).

That is, GameSchedule deals with a number of objects that broadly belong to Games class, in the sense that they do have common functions that are being accessed; at the same time, there is more granularity at which they are to be handled.

We would like to come up with a design that avoids unsafe downcasting, and is extensible in the sense that creating many subclasses under Games or any of its existing subclasses must not necessitate the addition of too much code to handle this requirement.

Examples:

  • A clumsy solution that I came up with, that doesn't do any downcasting at all, is to have dummy functions in the Game class for every subclass specific function that has to be called from GameSchedule. These dummy functions will have an overriding implementation in the appropriate subclasses which actually require its implementation.

  • We could explicitly maintain different containers for various subclasses of Games instead of a single container. But this would require a lot of extra code in GameSchedule, when the number of subclasses grow. Especially if we need to iterate through all the Games objects.

Is there a neat way of doing this?

Note: the code is written in C++

Update# 1: I realized that the question can be put in a much simpler way. Is it possible to have a container class for any object belonging to a hierarchy of classes? Moreover, this container class must have the ability to pick elements belonging to (or derive from) a particular class from the hierarchy and return an appropriate list.

In the context of the above problem, the container class must have functions like GetCricketGames, GetTestCricketGames, GetBaseballGame etc.,

도움이 되었습니까?

해결책

This is exactly one of the problems that The "Tell, Don't Ask" principle was created for.

You're describing an object that holds onto references to other objects, and wants to ask them what type of object they are before telling them what they need to do. From the article linked above:

The problem is that, as the caller, you should not be making decisions based on the state of the called object that result in you then changing the state of the object. The logic you are implementing is probably the called object’s responsibility, not yours. For you to make decisions outside the object violates its encapsulation.

If you break the rules of encapsulation, you not only introduce the runtime risks incurred by rampant downcasts, but also make your system significantly less maintainable by making it easier for components to become tightly coupled.


Now that that's out there, let's look at how the "Tell, Don't Ask" could be applied to your design problem.

Let's go through your stated constraints (in no particular order):

  1. GameSchedule needs to iterate over all games, performing general operations
  2. GameSchedule needs to iterate over a subset of all games (e.g., Basketball), to perform type-specific operations
  3. No downcasts
  4. Must easily accommodate new Game subclasses

The first step to following the "Tell, Don't Ask" principle is identifying the actions that will take place in the system. This lets us take a step back and evaluate what the system should be doing, without getting bogged down into the details of how it should be doing it.

You made the following comment in @MarkB's answer:

If there's a TestCricket class inheriting from Cricket, and it has many specific attributes concerning the timings of the various innings of the match, and we would like to initialize the values of all TestCricket objects' timing attributes to some preset value, I need a loop that picks all TestCricket objects and calls some function like setInningTimings(int inning_index, Time_Object t)

In this case, the action is: "Initialize the inning timings of all TestCricket games to a preset value."

This is problematic, because the code that wants to perform this initialization is unable to differentiate between TestCricket games, and other games (e.g., Basketball). But maybe it doesn't need to...

Most games have some element of time: Basketball games have time-limited periods, while Baseball games have (basically) innings with basically unlimited time. Each type of game could have its own completely unique configuration. This is not something we want to offload onto a single class.

Instead of asking each game what type of Game it is, and then telling it how to initialize, consider how things would work if the GameSchedule simply told each Game object to initialize. This delegates the responsibility of the initialization to the subclass of Game - the class with literally the most knowledge of what type of game it is.

This can feel really weird at first, because the GameSchedule object is relinquishing control to another object. This is an example of the Hollywood Principle. It's a completely different way of solving problems than the approach most developers initially learn.

This approach deals with the constraints in the following ways:

  1. GameSchedule can iterate over a list of Games without any problem
  2. GameSchedule no longer needs to know the subtypes of its Games
  3. No downcasting is necessary, because the subclasses themselves are handling the subclass-specific logic
  4. When a new subclass is added, no logic needs to be changed anywhere - the subclass itself implements the necessary details (e.g., an InitializeTiming() method).

Edit: Here's an example, as a proof-of-concept.

struct Game
{
    std::string m_name;

    Game(std::string name)
        : m_name(name)
    {
    }

    virtual void Start() = 0;
    virtual void InitializeTiming() = 0;
};

// A class to demonstrate a collaborating object
struct PeriodLengthProvider
{
    int GetPeriodLength();
}

struct Basketball : Game
{
    int m_period_length;
    PeriodLengthProvider* m_period_length_provider;

    Basketball(PeriodLengthProvider* period_length_provider)
        : Game("Basketball")
        , m_period_length_provider(period_length_provider)
    {
    }

    void Start() override;

    void InitializeTiming() override
    {
        m_period_length = m_time_provider->GetPeriodLength();
    }
};

struct Baseball : Game
{
    int m_number_of_innings;

    Baseball() : Game("Baseball") { }

    void Start() override;

    void InitializeTiming() override
    {
        m_number_of_innings = 9;
    }
}


struct GameSchedule
{
    std::vector<Game*> m_games;

    GameSchedule(std::vector<Game*> games)
        : m_games(games)
    {
    }

    void StartGames()
    {
        for(auto& game : m_games)
        {
           game->InitializeTiming();
           game->Start();
        }
    }
};

다른 팁

You've already identified the first two options that came to my mind: Make the base class have the methods in question, or maintain separate containers for each game type.

The fact that you don't feel these are appropriate leads me to believe that the "abstract" interface you provide in the Game base class may be far too concrete. I suspect that what you need to do is step back and look at the base interface.

You haven't given any concrete example to help, so I'm going to make one up. Let's say your basketball class has a NextQuarter method and hockey has NextPeriod. Instead, add to the base class a NextGameSegment method, or something that abstracts away the game-specific details. All the game-specific implementation details should be hidden in the child class with only a game-general interface needed by the schedule class.

C# supports reflections and by using the "is" keyword or GetType() member function you could do these easily. If you are writing your code in unmanaged C++, I think the best way to do this is add a GetType() method in your base class (Games?). Which in its turn would return an enum, containing all the classes that derive from it (so you would have to create an enum too) for that. That way you can safely determine the type you are dealing with only through the base type. Below is an example:

enum class GameTypes { Game, Basketball, Football, Hockey };

class Game
{
public:
    virtual GameTypes GetType() { return GameTypes::Game; }
}

class BasketBall : public Game
{
public:
    GameTypes GetType() { return GameTypes::Basketball; }
}

and you do this for the remaining games (e.g. Football, Hockey). Then you keep a container of Game objects only. As you get the Game object, you call its GetType() method and effectively determine its type.

You're trying to have it all, and you can't do that. :) Either you need to do a downcast, or you'll need to utilize something like the visitor pattern that would then require you to do work every time you create a new implementation of Game. Or you can fundamentally redesign things to eliminate the need to pick the individual Basketballs out of a collection of Games.

And FWIW: downcasting may be ugly, but it's not unsafe as long as you use pointers and check for null:

for(Game* game : allGames)
{
    Basketball* bball = dynamic_cast<Basketball*>(game);
    if(bball != nullptr)
        bball->SetupCourt();
}

I'd use the strategy pattern here.

Each game type has its own scheduling strategy which derives from the common strategy used by your game schedule class and decouples the dependency between the specific game and game schedule.

라이센스 : CC-BY-SA ~와 함께 속성
제휴하지 않습니다 StackOverflow
scroll top