Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Dealing with functions in a class that should be broken down into functions for clarity?

Tags:

c++

oop

How is this situation usually dealt with. For example, an object may need to do very specific things:

class Human
{
   public:
   void eat(Food food);
   void drink(Liquid liquid);
   String talkTo(Human human);
}

Say that this is what this class is supposed to do, but to actually do these might result in functions that are well over 10,000 lines. So you would break them down. The problem is, many of those helper functions should not be called by anything other than the function they are serving. This makes the code confusing in a way. For example, chew(Food food); would be called by eat() but should not be called by a user of the class and probably should not be called anywhere else.

How are these cases dealt with generally. I was looking at some classes from a real video game that looked like this:

class CHeli (7 variables, 19 functions)
Variables list

    CatalinaHasBeenShotDown
    CatalinaHeliOn
    NumScriptHelis
    NumRandomHelis
    TestForNewRandomHelisTimer
    ScriptHeliOn
    pHelis

Functions list

    FindPointerToCatalinasHeli (void)
    GenerateHeli (b)
    CatalinaTakeOff (void)
    ActivateHeli (b)
    MakeCatalinaHeliFlyAway (void)
    HasCatalinaBeenShotDown (void)
    InitHelis (void)
    UpdateHelis (void)
    TestRocketCollision (P7CVector)
    TestBulletCollision (P7CVectorP7CVectorP7CVector)
    SpecialHeliPreRender (void)
    SpawnFlyingComponent (i)
    StartCatalinaFlyBy (void)
    RemoveCatalinaHeli (void)
    Render (void)
    SetModelIndex (Ui)
    PreRenderAlways (void)
    ProcessControl (void)
    PreRender (void)

All of these look like fairly high level functions, which mean their source code must be pretty lengthy. What is good about this is that at a glance it is very clear what this class can do and the class looks easy to use. However, the code for these functions might be quite large.

What should a programmer do in these cases; what is proper practice for these types of situations.

like image 723
jmasterx Avatar asked Dec 17 '22 07:12

jmasterx


2 Answers

For example, chew(Food food); would be called by eat() but should not be called by a user of the class and probably should not be called anywhere else.

Then either make chew a private or protected member function, or a freestanding function in an anonymous namespace inside the eat implementation module:

// eat.cc

// details of digestion
namespace {
    void chew(Human &subject, Food &food)
    {
        while (!food.mushy())
            subject.move_jaws();
    }
}

void Human::eat(Food &food)
{
    chew(*this, food);
    swallow(*this, food);
}

The benefits of this approach compared to private member functions is that the implementation of eat can be changed without the header changing (requiring recompilation of client code). The drawback is that the function cannot be called by any function outside of its module, so it can't be shared by multiple member functions unless they share an implementation file, and that it can't access private parts of the class directly.

The drawback compared to protected member functions is that derived classes can't call chew directly.

like image 106
Fred Foo Avatar answered Dec 31 '22 02:12

Fred Foo


The implementation of one member function is allowed to be split in whatever way you want.

A popular option is to use private member functions:

struct Human
{
    void eat();

private:
    void chew(...);
    void eat_spinach();
    ...
};

or to use the Pimpl idiom:

struct Human
{
    void eat();
private:
    struct impl;
    std::unique_ptr<impl> p_impl;
};

struct Human::impl { ... };

However, as soon as the complexity of eat goes up, you surely don't want a collection of private methods accumulating (be it inside a Pimpl class or inside a private section).

So you want to break down the behavior. You can use classes:

struct SpinachEater
{
    void eat_spinach();
private:
    // Helpers for eating spinach
};

...

void Human::eat(Aliment* e)
{
    if (e->isSpinach()) // Use your favorite dispatch method here
                        // Factories, or some sort of polymorphism
                        // are possible ideas.
    {
        SpinachEater eater;
        eater.eat_spinach();
    }

    ...
}

with the basic principles:

  • Keep it simple
  • One class one responsibility
  • Never duplicate code

Edit: A slightly better illustration, showing a possible split into classes:

struct Aliment;

struct Human
{
    void eat(Aliment* e);

private:
    void process(Aliment* e); 
    void chew();
    void swallow();
    void throw_up();
};

// Everything below is in an implementation file
// As the code grows, it can of course be split into several
// implementation files.
struct AlimentProcessor
{
    virtual ~AlimentProcessor() {}
    virtual process() {}
};

struct VegetableProcessor : AlimentProcessor
{
private:
    virtual process() { std::cout << "Eeek\n"; }
};

struct MeatProcessor
{
private:
    virtual process() { std::cout << "Hmmm\n"; }
};

// Use your favorite dispatch method here.
// There are many ways to escape the use of dynamic_cast,
// especially if the number of aliments is expected to grow.
std::unique_ptr<AlimentProcessor> Factory(Aliment* e)
{
    typedef std::unique_ptr<AlimentProcessor> Handle;

    if (dynamic_cast<Vegetable*>(e)) 
        return Handle(new VegetableProcessor);
    else if (dynamic_cast<Meat*>(e))
        return Handle(new MeatProcessor); 
    else
        return Handle(new AlimentProcessor);
};

void Human::eat(Aliment* e)
{
    this->process(e);
    this->chew();

    if (e->isGood()) this->swallow();
    else this->throw_up();
}

void Human::process(Aliment* e)
{
    Factory(e)->process();
}
like image 23
Alexandre C. Avatar answered Dec 31 '22 01:12

Alexandre C.