Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Posn shifting member functions don't meet expectations #6

Open
brghena opened this issue Nov 29, 2021 · 1 comment
Open

Posn shifting member functions don't meet expectations #6

brghena opened this issue Nov 29, 2021 · 1 comment

Comments

@brghena
Copy link
Contributor

brghena commented Nov 29, 2021

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
Copy link
Owner

tov commented Nov 29, 2021

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants