Eliminate axis-aligned bounding box (AABB) variables

I have an axis-aligned bounding box (AABB) that I've coded, but at the moment it is very redundant. I feel like there are variables that I can get rid of. I need the AABB to be very fast, and I'd like it to be more lightweight than it is.

```
#pragma once

#include "Engine.h"
// Collision
#include "Vec2D.h"
// End Collision

#include "Renderer.h"

struct ENGINE_API AABB {
private:
Vec2D min;
Vec2D max;

double x;
double y;
double width;
double height;
double xwidth; // x + width
double yheight; // y + height

double vertMid; // vertical mid, cached for quadtree
double horzMid; // horizontal mid, cached for quadtree

// Normalize
void calcMid() {
vertMid = x + (width / 2);
horzMid = y + (height / 2);
xwidth = x + width;
yheight = y + height;
}

void init(Vec2D, Vec2D);
// End Normalize
public:
// Constructor
AABB(Vec2D min, Vec2D max) {
init(min, max);
}

AABB(double x, double y, double width, double height){
init(Vec2D(x, y), Vec2D(x + width, y + height));
}
// End Constructor

// Getters
double getX()const {
return x;
}

double getY()const {
return y;
}

double getWidth()const {
return width;
}

double getHeight()const {
return height;
}

double getHorzMid()const {
return horzMid;
}

double getVertMid()const {
return vertMid;
}

double getXWidth()const {
return xwidth;
}

double getYHeight()const {
return yheight;
}
// End Getters

// Base
bool collides(AABB other) {
if (xwidth other.xwidth) { return false; }
if (yheight other.yheight) { return false; }

return true;
}

bool contains(Vec2D point) {
if (x > point.getX() || xwidth point.getY() || yheight x = x;
this->y = y;
calcMid();
}
// End Base
// Testing
void render

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).

StackExchange Code Review Q#45323, answer score: 4