6
\$\begingroup\$

I have a few constants in a game that I'm doing for hobby. I need to store constants for the total size of the a Physical World and the size of the Screen.

I can position the world's objects to the screen or vice versa.

To position the images on the screen for example I get the position that the body occupies in the physical world and transform to the screen by doing:

(Full Screen Size / Total size of the World) * position of the object in the world.

I came up with an ingenious way to keep these constant while I do conversion operations using the enum below:

import com.badlogic.gdx.math.Vector2;
import static br.com.games.Test.Sizes.*;

public class Test {

    public static enum Sizes {
        WORLD(new Vector2(25.806f, 15.48f)), SCREEN(new Vector2(800f, 480f)), TRANSFORM(
                new Vector2(-1, -1));

        private Vector2 value;

        private Sizes(Vector2 value) {
            this.value = value;
        }

        private Vector2 inputValue;
        private Vector2 sourceValue;

        public Sizes value(Vector2 value) {
            inputValue = value;
            return this;
        }

        public Sizes from(Sizes size) {
            sourceValue = size.value;
            return this;
        }

        public Vector2 to(Sizes size) {
            float x = size.value.x / sourceValue.x * inputValue.x;
            float y = size.value.y / sourceValue.y * inputValue.y;
            return Vector2.Zero.set(x, y);
        }

        public Vector2 getValue() {
            return value;
        }
    }

}

I can use it this way:

    public static void main(String[] args) {
        TRANSFORM.value(new Vector2(10, 10)).from(WORLD)
                .to(SCREEN);
    }

Vector2 class is here.

What do you think of this way of using constants and operations? Do you see any problems? Could you point out some improvement?

\$\endgroup\$
6
\$\begingroup\$

I'm not at all sold on TRANSFORM being part of your enum. The API for this enum lets you make nonsense calls like WORLD.from(WORLD).to(TRANSFORM). Always try to prevent invalid use of your APIs. I'm not really sure why you wouldn't use a naive approach like two static methods Transformer.worldToScreen(Vector2) and Transformer.screenToWorld(Vector2). Are you anticipating needing additional conversions later?

If you're really hankering for a fluent API for some reason, you could do Transform.value(Vector2D).from(Sizes).to(Sizes). You can control the return types to prevent at least some of the possible errors, but there are still some invalid calls like Transform.value(...).from(WORLD).to(WORLD).

**** EDIT ****

Since you're interested in the fluent approach, here's something to get you started. Obviously it needs documentation and input checking. The DisplayType enum can either belong to Transform or it can be top-level, depending on your needs. Likewise, the two inner classes can easily be split out.

As far as not instantiating objects, you can't really do a fluent API correctly without new objects. Do you have a real, demonstrated performance issue with creating new objects? Many people wrongly assume that object creation and garbage collection will be a big issue without testing.

If you really need to do this without new object instances, then your only good option as I see it is static methods. Your Sizes enum is too easy to use incorrectly.

public final class Transform {

    public static enum DisplayType {        
        SCREEN(new Vector2(800f, 480f)),
        WORLD(new Vector2(25.806f, 15.48f));

        private final Vector2 size;

        private DisplayType(final Vector2 size) {
            this.size = size;
        }
    }

    private Transform() {
        super();
    }

    public static From value(final Vector2 value) {
        return new From(value);
    }

    public static final class From {

        private final Vector2 value;

        From(final Vector2 value) {
            this.value = value;
        }

        public To from(final DisplayType sourceDisplayType) {
            return new To(this.value, sourceDisplayType);
        }
    }

    public static final class To {

        private final Vector2 value;
        private final DisplayType from;

        To(final Vector2 value, final DisplayType from) {
            this.value = value;
            this.from = from;
        }

        public Vector2 to(final DisplayType targetDisplayType) {
            /* TODO: Do math here */
            throw new UnsupportedOperationException();
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ There should be other conversions in the future. I can have different screen sizes, for example, the menu screens have another resolution. Different stages of the game may also have different resolutions. I like your second suggestion, the only problem is that it would need to instantiate the class Transform in the value () method. As these conversions run in the main game loop we need to avoid new class instances. I edited the original code and created a version using a static instance of Transform but not evaluated yet if this can be problematic or not... \$\endgroup\$ – alexpfx Apr 30 '15 at 4:02
1
\$\begingroup\$

I totally understand how exciting it is to make full use of the richness of the Java language but sometimes you must stand back and decide whether you have don good or bad. Here I think (and this is only my opinion) you have not done good.

My problem with this is that it is not obvious how to use the mechanism. I would suggest that the transform mechanism should not reside in the Sizes enum, it should be in the Vector.

Something like:

public interface Size {
    public float getX();
    public float getY();
}

public static enum Sizes implements Size {

    WORLD(new Vector2(25.806f, 15.48f)),
    SCREEN(new Vector2(800f, 480f));

    private Vector2 value;

    private Sizes(Vector2 value) {
        this.value = value;
    }

    public float getX() {
        return value.getX();
    }
    public float getY() {
        return value.getY();
    }

}

private static class Vector2 implements Size {

    final float x;
    final float y;

    public Vector2(float x, float y) {
        this.x = x;
        this.y = y;
    }

    @Override
    public float getX() {
        return x;
    }

    @Override
    public float getY() {
        return y;
    }

    public Size transform(Size from, Size to) {
        return new Vector2(x / from.getX() * to.getX(), y / from.getY() * to.getY());
    }
}

Your sample transform would then look like:

public void test() {
    Size onScreen = new Vector2(1,1).transform(Sizes.WORLD, Sizes.SCREEN);
}

If (as suggested by @h.j.k) you are not able to adjust Vector2 you could still achieve the same functionality with something like:

class Point implements Size {

    private final Vector2 value;

    public Point(Vector2 value) {
        this.value = value;
    }

    public Point(float x, float y) {
        this.value = new Vector2(x, y);
    }

    @Override
    public float getX() {
        return value.x;
    }

    @Override
    public float getY() {
        return value.y;
    }

    public Size transform(Size from, Size to) {
        return new Vector2(value.getX() / from.getX() * to.getX(), value.getY() / from.getY() * to.getY());
    }
}
\$\endgroup\$
  • \$\begingroup\$ I believe Vector2 is an external class... \$\endgroup\$ – h.j.k. Apr 30 '15 at 9:19
  • \$\begingroup\$ @h.j.k. - In that case a new class should be used that wraps Vector2 and implements the Size interface and implements transform. \$\endgroup\$ – OldCurmudgeon Apr 30 '15 at 9:21
  • \$\begingroup\$ @alexpfsv- your loss. \$\endgroup\$ – OldCurmudgeon Apr 30 '15 at 23:59
  • \$\begingroup\$ I agree that the use of Transform constant along with the other confuses everything, but.i don't like to much encapsulation you did. I think the responsability to transform belongs to Screen and not to Vector2. Vector is has responsibility for scale, sum, subtract, etc. I think the call of your API Size onScreen = new Vector2(1,1).transform(Sizes.WORLD, Sizes.SCREEN); is better, but still is not entirely clear. \$\endgroup\$ – alexpfx May 1 '15 at 0:03
0
\$\begingroup\$

I ended up implementing a new version based on the notes that Eric did (prior to editing), but modifying some things to avoid at all costs creating objects. So I created a static variable to serve as temporary variable to perform the calculations. I think the call syntax was as I expected. I also changed the enum name to ResolutionConstants. I think DisplayType to be good, but since not all resolutions are meant to display information on a screen think this way is better.

package gists.constants;

import com.badlogic.gdx.math.Vector2;

/**
 * Created by alexandre on 30/04/15.
 */
public enum ResolutionConstants {
    WORLD(new Vector2(25.806f, 15.48f)), SCREEN(new Vector2(800f, 480f));

    private Vector2 value;
    private Vector2 inputValue;
    private Vector2 sourceValue;

    private ResolutionConstants(Vector2 value) {
        this.value = value;
    }

    /**
     * Alternative Transform method.
     */
    public static class Transform {
        private Vector2 inputValue;
        private Vector2 sourceValue;

        private final static Transform instance = new Transform();

        private Transform() {

        }

        public static Transform value(Vector2 inputValue) {
            instance.inputValue = inputValue;
            return instance;
        }

        public Transform from(ResolutionConstants size) {
            this.sourceValue = size.value;
            return this;
        }

        public Vector2 to(ResolutionConstants size) {
            float x = size.value.x / sourceValue.x * inputValue.x;
            float y = size.value.y / sourceValue.y * inputValue.y;
            return Vector2.Zero.set(x, y);
        }

        public static void main(String[] args) { // Test
            final Vector2 transfomed = Transform.value(new Vector2(10, 10)).from(WORLD).to(SCREEN);
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.