Request: Point::translate(x, y)

#1

Hi Jules,

Noticed that Point has “translated,” but not “translate.” Would like to request the addition of this function. (Doing also so makes it consistent with Rectangle)

/** Moves the position by adding amount to its x and y co-ordinates. */
void translate (ValueType deltaX, ValueType deltaY) const noexcept  { x += deltaX; y += deltaY; }

Edit: Noticed Point::addXY() - does what I’m looking for… but why not use “translate”?

#2

(TBH I didn’t realise that addXY() was in there, and it should probably be removed).

For such a simple class, it’s better practice to treat it like an immutable primitive, so “p += Point (x, y)” or “p = p.translated (x, y)” is how I’d encourage you to write it. There’s really no need for a mutating translation op.

#3

Okay… so with immutability in mind, why expose x and y?

Edit: And with that you said, why have so many mutators available? e.g.: setX(), setY(), setXY(), various operators… Seems strangely contradictory!

#4

Yes, fair point! Not sure why so many of those are in there, it’s an old class!

#5

That’s fine. My next question is; what’s the advantage of treating such an object as immutable? Isn’t there more generated code involved in doing so (constructor, with operator calls? though inlining would most likely happen (I’d hope)), and don’t you sacrifice clarity?

#6

It just makes your code more functional in style, so less danger of side-effects, easier for the compiler to optimise, (and all the usual pro-functional arguments)

#7

please dont privatize x & y. It will make a lot of coordinate calculation code look just horrid.

also, not harm to leave get & set versions there already, for consistency.

my 2c.

#8

Don’t worry, I like having x & y exposed, and am not planning on removing any methods, as it’d just annoy people who’ve used them.