Reduce CopyPaste of addElement() in Screen & Element

Hey, i noticed that you recently added the function addElement(Element element, boolean hide) :
However it’s just copy pasta of the old method addElement(Element element) with added

[java]
if (hide)
element.hide();
[/java]

So to reduce copy pasta just call the new method in the old method with a default value:

[java]
@Override
public void addElement(Element element) {
addElement(element,false);
}
[/java]

No more copy pasta, both methods use the same code, less maintaining :slight_smile:

1 Like
@h1ghst0r3y said: Hey, i noticed that you recently added the function addElement(Element element, boolean hide) : However it's just copy pasta of the old method addElement(Element element) with added

[java]
if (hide)
element.hide();
[/java]

So to reduce copy pasta just call the new method in the old method with a default value:

[java]
@Override
public void addElement(Element element) {
addElement(element,false);
}
[/java]

No more copy pasta, both methods use the same code, less maintaining :slight_smile:

Doh! I’ll update this… or have the new method call the prior or something :wink:

Naaah, then you’re just adding functionality after the run of the first method and may have to do checks or stuff again.

The if(hide) is inside another if:

[java]
if (screen.getElementById(child.getUID()) != null) {

} else {

if (hide)
child.hide();
}
[/java]

This code would behave differently:

[java]

@Override
public void addElement(Element element, boolean hide) {
    addElement(element);
    if(hide)
	child.hide();

}

[/java]

than:

[java]

@Override
public void addElement(Element element) {
    addElement(element,false);
}

[/java]

Because the

[java]

	if (screen.getElementById(child.getUID()) != null) {

[/java]

wouldn’t be checked.

Quite difficult to describe =D

Easiest way is to “abandon” the old method so it calls the new method with the newly added parameters.
This way both methods have the same codebase.

1 Like

True enough! I’ll get this changed tonight. Errand and dinner for family first though :wink:

Thanks a bunch!

While you are at it, Screen’s getEventElement, getContactElement and getTargetElement have some serious pasta in them. The code

[java]guiRayOrigin.set(x, y, 0f);

elementZOrderRay.setOrigin(guiRayOrigin);
results.clear();

t0neg0dGUI.collideWith(elementZOrderRay, results);

lastCollision = results.getClosestCollision();

float z = 0;
Element testEl = null, el = null;
for (CollisionResult result : results) {
boolean discard = false;
if (result.getGeometry().getParent() instanceof Element) {
testEl = ((Element)(result.getGeometry().getParent()));
if (testEl.getIgnoreMouse()) {
discard = true;
} else if (testEl.getIsClipped()) {
if (result.getContactPoint().getX() < testEl.getClippingBounds().getX() ||
result.getContactPoint().getX() > testEl.getClippingBounds().getZ() ||
result.getContactPoint().getY() < testEl.getClippingBounds().getY() ||
result.getContactPoint().getY() > testEl.getClippingBounds().getW()) {
discard = true;
}
}
}
if (!discard) {
if (result.getGeometry().getParent() instanceof Element) {
el = testEl;
}
}
}[/java]

is in all 3 of them, with very small differences.

@t0neg0d If you want, I could point out a lot of places where I think the code could be better.

Of course… feel free to point out any and all.

The library was mostly put together over a fairly short period of time and could use a bit (heh) of cleaning up.