search

tov / ge211

A relatively simple C++ student game engine
https://tov.github.io/ge211/
6 stars 3 forks source link

Posn shifting member functions don't meet expectations #6

Open brghena opened 2 years ago

brghena commented 2 years ago

The up_by(), down_by(), etc. "shifting member functions" for Posn construct a new Posn modified from the existing one as specified. I've found that this doesn't match student's expectations that they will modify the Posn they are called on.

It also doesn't match my own expectations. If they were named from_up_by() similarly to the functions of Rect, I think that they would make more sense. Or if they actually modified they Posn they are called on.

Is it intentional behavior that the shifting member functions create a new Posn rather than modifying the one they are called on?

tov commented 2 years ago

Yes, this was the intended behavior. I think the intent was for them to get some practice with a non-mutating API like this. But if you think it’s too confusing, I would be open to one of a few different changes:

  1. We could rename the functions from up_by to above_by.

  2. We could change the signatures from

        Posn up_by(Coord) const;

    to

        Posn& up_by(Coord);

  3. We could get rid of them entirely, since they’re mostly redundant with Posn Posn::operator+(Dims) const and Posn& Posn::operator+(Dims).

What do you think?