5
\$\begingroup\$

I have an array of a instances of an enum type. A random double variable is initialized with a value between 0.... to 1, and should be used to get the closest value's enum instance to that randomly set double value.

The enum looks like this:

public enum TargetScope {

    CLEAN(1, 5040),
    FIRST(.888, 5046),
    SECOND(.777,5052),
    THIRD(.666,5058),
    FOURTH(.555, 5064),
    FIFTH(.444, 5070),
    SIXTH(.333, 5076),
    SEVENTH(.222, 5082),
    EIGHTH(.001, 5088);

    private int interfaceBase;
    private double time;

    TargetScope(double time,int interfaceIdBase) {
        this.time = time;
        this.interfaceBase = interfaceIdBase;
    }

    public int getTime() {
        return (int) this.time * TargetManager.TARGET_SEARCH_WAIT;
    }

    public int getInterfaceBase() {
        return this.interfaceBase;
    }

    public double getPercent() {
        return this.time;
    }
}

I have implemented an algorithm which does that, using the array and few temp variables:

public TargetScope getInterfaceScopeByTime() {
    double alpha = (double) this.time / TargetManager.TARGET_SEARCH_WAIT;

    TargetScope[] scopes = TargetScope.values();

    double delta = 1;
    TargetScope scope = null;

    for (int i = 0; i < scopes.length; i++) {
        double distance = Math.abs(scopes[i].getPercent() - alpha);
        if (distance < delta) {
            delta = distance;
            scope = scopes[i];
        }
    }

    return scope;
}

Basically it gets the closest Enum.getPercent() value in the list to alpha, and then returns the final closest TargetScope instance.

But I really feel bad about using this, where Java has an advanced collection API, which can be used better for this case, but I am not really as experienced with Collections API to use them for that.

Is there really a shorter way to perform this?

The idea behind this

I am working on a game-server plugin, which basically makes two players targets and have them fight each other. For a player to get a target, he needs his target-timer to hit 0, in order to let the system search for a target. Originally, the default target timer is 1000, each system cycle is 600ms, so 1000 is 10 minutes.

Each player has an interface that looks like a sliced circle to 8 pieces, something like this:

img

I need to let the interface know, what slices should be filled with colour, to let the player know when he can get his target. a fully coloured scope (circle) means that the timer hit 0.

Each interface slice has it's own id, as you see in the enum, int interfaceBase.

I need to update the player's interface, by using the player's current left time.

So if his time is 800, I do 800 / 1000, I get 0.8, now from the enums list, the closest enum is called SECOND because the closest value to 0.8 out of the list is 0.777, therefore when the server updates player's interface exactly when the timer is 800, then the interface will display one slice out of 8.

The interface updates every 10 seconds, per player (as in each player has its executor).

\$\endgroup\$
  • \$\begingroup\$ Usually code snippets are not best practice to post here. because we have to take extra effort first to make it runnable.(you could have given class with main method or couple of Junits ). I have covered this point in my answer also. \$\endgroup\$ – Shirishkumar Bari Sep 4 '14 at 18:29
3
\$\begingroup\$

This problem can be solved mathematically much better than it can with a list, or collection.

What you have is 9 states, and a timer that counts down. The timer can be expressed as a proportion of 1. Because the states are all equally spaced on 1/9 intervals, you can just do math....

So, if the time is \$x\$ and the total time is \$y\$, then the current portion is \$\frac{x}{y}\$ If you multiply this value by 9, you get something on the scale of 0 to 9 inclusive. you really want:

public int tileID(int currentTime, int totalTime, int tileCount) {
    // we want the shift to happen at less than half-the-time (in the middle of the period).
    double shift = (1.0 / tileCount) / 2;
    double progress = (double)currentTime / (double)(totalTime);
    int tile = (int)(progress * tileCount + shift);
    return tile;
}

shift is needed to make the (int) truncation work. Your comment has made me think, and, in reality, it is not the most readable/understandable code. In fact it is broken, and shift should just be 0.5 always... Let me re-do it in the form of a round() instead of a truncation:

public int tileID(int currentTime, int totalTime, int tileCount) {
    double progress = (double)currentTime / (double)totalTime;
    int tile = (int)Math.round(progress * tileCount + 0.3);
    // because your tiles are in the opposite order: tileCount - 1 - tile
    return tileCount - 1 - tile;
}

Then, you can use this with:

TargetScope tile = TargetScope.values[tileId(this.time,
              TargetManager.TARGET_SEARCH_WAIT, TargetScope.values[].length)];
\$\endgroup\$
  • \$\begingroup\$ Nice answer, but what is shift exactly? \$\endgroup\$ – user3123545 Sep 4 '14 at 17:57
  • \$\begingroup\$ @user3123545 - good comment, I have edited my answer. \$\endgroup\$ – rolfl Sep 4 '14 at 18:11
  • \$\begingroup\$ Now the answer looks very logical, because the shift looked a bit confusing, about why it's needed. Thanks! \$\endgroup\$ – user3123545 Sep 4 '14 at 18:24
  • \$\begingroup\$ There's a small problem with your method. Let's say the user got 300 ticks left to get his target out of 1000. 300 / 1000 = 0.3, now 0.3 * 9 = 2.7, rounded 3. this means it will get the THIRD enum. Okay no problem, I did tiles - progress * tiles, but now I get full circle (as in it enters the 9th enum before the timer ends ( I had 200ticks left ). This is probably because you also have to wait in the lat enum, like rest of enums. Is there a mathematical way to solve it without if statement check? \$\endgroup\$ – user3123545 Sep 4 '14 at 18:59
  • \$\begingroup\$ Just a side note, my enums are ordered from 0 slices to 8 slices. \$\endgroup\$ – user3123545 Sep 4 '14 at 18:59
1
\$\begingroup\$

For-each loop

My Java is somewhat rusty but I think you could use a for-each loop to write something like :

for (TargetScope s: TargetScope.values())
{
    double distance = Math.abs(s.getPercent() - alpha);
    if (distance < delta) {
        delta = distance;
        scope = s;
    }
}

Code organisation

To be honest with you, I have no idea what your code is supposed to be doing. However, wouldn't it make sense to define a method getInterfaceScopeByTime() taking the time you want to approximate as an argument instead of using the attribute time? It would make things somewhat easier to test/understand and you can always call the method with this.time if required.

Algorithm

From an algorithm point of view, depending on the number of times you plan to do this, you might find interesting to perform some preprocessing whose cost will be amortised to achieve better average performances. Among the different options. For instance, you can sort the values : if at a given step, the computed distance is bigger than the minimal distance, it means that you are getting away from the closest value and you can stop iterating. Otherwise, you can use a binary search to find the closest value.

\$\endgroup\$
  • \$\begingroup\$ Would it help you improve the answer, if I gave you a background on what I am going to do with this code, as in use it for? \$\endgroup\$ – user3123545 Sep 4 '14 at 14:23
  • \$\begingroup\$ Most probably. If it doesn't help me, it might help someone else. \$\endgroup\$ – SylvainD Sep 4 '14 at 14:35
  • \$\begingroup\$ Added extra info. \$\endgroup\$ – user3123545 Sep 4 '14 at 14:48
1
\$\begingroup\$

improvements:

  1. As rightly pointed by Josay You could use the forEach for iterating over Enum

  2. Your few method names can be changed like getInterfaceScopeByTime should be renamed to getInterfaceScopeByNearestTime

  3. in your ENUM getPercent() seems to be unnecessary and it is adding burden on alpha while subtracting it from percentTime() you have to divide the actual value by a constant TARGET_SEARCH_WAIT intstead use your getTime() for substracting alpha from it .

  4. Usually code snippets are not best practice to post here. because we have to take extra effort first to make it runnable.(you could have given class with main method or couple of Junits )

  5. In your shared both methods getInterfaceBase() is not used anywhere so we can remove that .but i suppose you need it somewhere else hence I have kept in below code

  6. Variable is named as interfaceBase and the parameter to constructor is named as interfaceIdBase. it creates confusion. you could use name interfaceBaseId for both.

BUG :

  • your getTime() in most cases returns 0 . because you placed int at wrong place .

With points mentioned above ,please find the modified code below

 package com.study.programs;

public class InterfaceScope {
    private double time= 800;
    public static void main(String args[]){
        InterfaceScope interfaceScope = new InterfaceScope();
        interfaceScope.time =800;
        System.out.println("For time = "+interfaceScope.time+" Nearest Scope ="+interfaceScope.getInterfaceScopeByNearestTime());
        interfaceScope.time =499;
        System.out.println("For time = " + interfaceScope.time + " Nearest Scope =" + interfaceScope.getInterfaceScopeByNearestTime());
        interfaceScope.time =500;
        System.out.println("For time = "+interfaceScope.time+" Nearest Scope ="+interfaceScope.getInterfaceScopeByNearestTime());
    }
    public TargetScope getInterfaceScopeByNearestTime() {
        double delta = 1000;
        TargetScope nearestScope = null;
        for (TargetScope scope :TargetScope.values()) {
            double distance = Math.abs(scope.getTime()-this.time);
            if (distance < delta) {
                delta = distance;
                nearestScope = scope;
            }
        }
        return nearestScope;
    }
}
enum TargetScope {
    CLEAN(1, 5040),
    FIRST(.888, 5046),
    SECOND(.777,5052),
    THIRD(.666,5058),
    FOURTH(.555, 5064),
    FIFTH(.444, 5070),
    SIXTH(.333, 5076),
    SEVENTH(.222, 5082),
    EIGHTH(.001, 5088);
    private static final int TARGET_SEARCH_WAIT = 1000;
    private int interfaceBaseId;
    private double time;

    TargetScope(double time,int interfaceBaseId) {
        this.time = time;
        this.interfaceBaseId = interfaceBaseId;
    }
    public int getTime() {
        return (int) (this.time * TARGET_SEARCH_WAIT);
    }

    public int getInterfaceBaseId() {
        return this.interfaceBaseId;
    }
}
\$\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.