tresinformal / drakkar

The tresinformal video game called 'Drakkar'
GNU General Public License v3.0
11 stars 4 forks source link

All classes that have a position now get that position from free logistic functions and not member functions. #411

Closed swom closed 2 years ago

swom commented 2 years ago

For classes:

Change all calls to member functions that read the m_xor m_ymember, i.e.:

a_class.get_x(); //or
a_class.get_y();

to free logistic functions, i.e.:

get_x(const a_class& a_class);
get_y(const a_class& a_class);

do this in both tests and the code. The only functions allowed to call member functions to read x and y

a_class.get_x(); //or
a_class.get_y();

are the free logistic functions you will have used everywhere else

get_x(const a_class& a_class);
get_y(const a_class& a_class);

To be noted:

  1. If a free_logistic function with the same name exists for the class DO NOT write a new one, or change the existing one, just swap it with calls to the member function.
  2. you SHALL NOT delete the definition and declaration of
    a_class.get_x(); //or
    a_class.get_y();

    from a class, keep it, just do not let anybody except the free logistic functions use it.

TheoPannetier commented 2 years ago

Would a template be a good idea here? I'm tempted to take advantage of the 'get_position()' function that all items share and do it all at once with

template <typename T>
double get_x(const T& item)
{ 
    return item.get_position.get_x();
}
template <typename T>
double  get_y(const T& item)
{ 
    return item.get_position.get_y();
}

or is some good ol' function overloading preferable?

richelbilderbeek commented 2 years ago

@TheoPannetier: this is a fun idea and I am happy you found it (and I could not help fix the code).

I would not mind gradually adding this to the project. It would be a good experience for some team members to experiment with templates.

Argument against doing so, is that no new functionality is gained (just existing one removed) and the code will be harder for newbies to understand.

I think with the current state of the team, I think the pros outweight the cons :-)

template <typename T>
double get_position(const T& item)
{ 
    return item.get_position();
}
template <typename T>
double get_x(const T& item)
{ 
    return get_position(item).get_x();
}
template <typename T>
double  get_y(const T& item)
{ 
    return get_position(item).get_y();
}