Return to Answer

In a comment in your other question you claimed,

Thank you, Caching vertMid and horzMid really helped.

Whether caching is helpful depends on:

• How often you used the cached data
• How tight your performance requirements are
• Whether your cache implementation is reliable

One of your comments says,

// vertical mid, cached for quadtree

Only your testing can show whether that optimization is helpful.

From a performance point of view, in general:

• If it's not used it's harmful to cache
• If it's used once then it's harmless/useless to cache
• If it's used several times then it's useful to cache

However making something bigger (by caching extra values) can also have some negative effect on performance, e.g. because you can't fit so many objects in the CPU cache (which is faster than memory): which is why you should profile/test to see whether caching makes it faster in practice as well as in theory.

If you want to eliminate duplicate variables, you only need 4 doubles:

• min and max
• or, x and either width or xwidth, and y and either height and yheight

I found your xwidth and yheight names confusing to read; I would have preferred 'right' and 'bottom'.

If you have large structs, you may improve performance by curing your habit of passing objects by value instead of by reference. For example, this ...

bool collides(AABB other)

... should be ...

bool collides(const AABB& other) const

You can also use different classes for different purposes. For example, the AABB bounds member of your Quadtree class needs getHorzMid() and getVertMid() members, but those methods aren't needed/used by the AABB which you return using Locatable::getAABB(). So you could use two classes, e.g. AABB (without getHorzMid and getVertMid members), and a SplittableAABB class (used as a member of Quadtree) which adds those members.

I don't much like AABB as a class name: partly because all-upper-case looks like a macro to me: consider Bounds or Rect or Rectangle instead.

Your current code creates but doesn't read the initialize the min and max data members.

Your move method doesn't update the data in these members (rendering that data untrue).

Maybe Vec2D min and Vec2D max should be (temporary) local variables of your init function, not (permanent) member data of the class.

The parameters passed to the AABB(Vec2D min, Vec2D max) constructor should probably be named one and two not min and max (and should possibly be passed by reference not by value).

In a comment in your other question you claimed,

Thank you, Caching vertMid and horzMid really helped.

Whether caching is helpful depends on:

• How often you used the cached data
• How tight your performance requirements are
• Whether your cache implementation is reliable

One of your comments says,

// vertical mid, cached for quadtree

Only your testing can show whether that optimization is helpful.

From a performance point of view, in general:

• If it's not used it's harmful to cache
• If it's used once then it's harmless/useless to cache
• If it's used several times then it's useful to cache

However making something bigger (by caching extra values) can also have some negative effect on performance, e.g. because you can't fit so many objects in the CPU cache (which is faster than memory): which is why you should profile/test to see whether caching makes it faster in practice as well as in theory.

If you want to eliminate duplicate variables, you only need 4 doubles:

• min and max
• or, x and either width or xwidth, and y and either height and yheight

I found your xwidth and yheight names confusing to read; I would have preferred 'right' and 'bottom'.

If you have large structs, you may improve performance by curing your habit of passing objects by value instead of by reference. For example, this ...

bool collides(AABB other)

... should be ...

bool collides(const AABB& other) const

You can also use different classes for different purposes. For example, the AABB bounds member of your Quadtree class needs getHorzMid() and getVertMid() members, but those methods aren't needed/used by the AABB which you return using Locatable::getAABB(). So you could use two classes, e.g. AABB (without getHorzMid and getVertMid members), and a SplittableAABB class (used as a member of Quadtree) which adds those members.

I don't much like AABB as a class name: partly because all-upper-case looks like a macro to me: consider Bounds or Rect or Rectangle instead.

Your current code creates but doesn't read the initialize the min and max data members.

Your move method doesn't update the data in these members (rendering that data untrue).

Maybe Vec2D min and Vec2D max should be (temporary) local variables of your init function, not (permanent) member data of the class.

The parameters passed to the AABB(Vec2D min, Vec2D max) constructor should probably be named one and two not min and max (and should possibly be passed by reference not by value).

In a comment in your other question you claimed,

Thank you, Caching vertMid and horzMid really helped.

Whether caching is helpful depends on:

• How often you used the cached data
• How tight your performance requirements are
• Whether your cache implementation is reliable

One of your comments says,

// vertical mid, cached for quadtree

Only your testing can show whether that optimization is helpful.

If you want to eliminate duplicate variables, you only need 4 doubles:

• min and max
• or, x and either width or xwidth, and y and either height and yheight

I found your xwidth and yheight names confusing to read; I would have preferred 'right' and 'bottom'.

If you have large structs, you may improve performance by curing your habit of passing objects by value instead of by reference. For example, this ...

bool collides(AABB other)

... should be ...

bool collides(const AABB& other) const

You can also use different classes for different purposes. For example, the AABB bounds member of your Quadtree class needs getHorzMid() and getVertMid() members, but those methods aren't needed/used by the AABB which you return using Locatable::getAABB(). So you could use two classes, e.g. AABB (without getHorzMid and getVertMid members), and a SplittableAABB class (used as a member of Quadtree) which adds those members.

I don't much like AABB as a class name: partly because all-upper-case looks like a macro to me: consider Bounds or Rect or Rectangle instead.

In a comment in your other question you claimed,

Thank you, Caching vertMid and horzMid really helped.

Whether caching is helpful depends on:

• How often you used the cached data
• How tight your performance requirements are
• Whether your cache implementation is reliable

One of your comments says,

// vertical mid, cached for quadtree

Only your testing can show whether that optimization is helpful.

From a performance point of view, in general:

• If it's not used it's harmful to cache
• If it's used once then it's harmless/useless to cache
• If it's used several times then it's useful to cache

However making something bigger (by caching extra values) can also have some negative effect on performance, e.g. because you can't fit so many objects in the CPU cache (which is faster than memory): which is why you should profile/test to see whether caching makes it faster in practice as well as in theory.

If you want to eliminate duplicate variables, you only need 4 doubles:

• min and max
• or, x and either width or xwidth, and y and either height and yheight

I found your xwidth and yheight names confusing to read; I would have preferred 'right' and 'bottom'.

If you have large structs, you may improve performance by curing your habit of passing objects by value instead of by reference. For example, this ...

bool collides(AABB other)

... should be ...

bool collides(const AABB& other) const

You can also use different classes for different purposes. For example, the AABB bounds member of your Quadtree class needs getHorzMid() and getVertMid() members, but those methods aren't needed/used by the AABB which you return using Locatable::getAABB(). So you could use two classes, e.g. AABB (without getHorzMid and getVertMid members), and a SplittableAABB class (used as a member of Quadtree) which adds those members.

I don't much like AABB as a class name: partly because all-upper-case looks like a macro to me: consider Bounds or Rect or Rectangle instead.

Your current code creates but doesn't read the initialize the min and max data members.

Your move method doesn't update the data in these members (rendering that data untrue).

Maybe Vec2D min and Vec2D max should be (temporary) local variables of your init function, not (permanent) member data of the class.

The parameters passed to the AABB(Vec2D min, Vec2D max) constructor should probably be named one and two not min and max (and should possibly be passed by reference not by value).