CODEWITHSUNDEEP
javastring
Zibi Smough
Zibi Smough

Reputation: 113

Key-Value map instead of a very long ELSE-IF construction?

I am currently working on a method that is mapping some strings to other strings.

It has a LOT of values, and the method starts to look like this:

The method ValueHelper.isEqual() is looking for an exact match.

private IValue1 mapValue(IValue2 value2) {
    if (ValueHelper.isEqual(value2.getName(), StatusValues.ACTIVE)) {
            return ValueHelper.getName(StatusValues2.WORKING);
    } else if (ValueHelper.isEqual(value2.getName(), StatusValues.INACTIVE)) {
            return ValueHelper.getName(StatusValues2.NOT_WORKING);
    } else if (ValueHelper.isEqual(value2.getName(), StatusValues.IN_SERVICE)) {
            return ValueHelper.getName(StatusValues2.SERVICE);
    }
}

At current point I have 10 else-if code blocks.

What is the best way to make this method simpler and shorter? Extracting the values to a Key-Value map? Or maybe another option?

Upvotes: 3

Views: 147

Answers (5)

sohan nohemy
sohan nohemy

Reputation: 615

Use guava Immutable Map. They are faster and take less memory than standard java HashMap.

Upvotes: -1

sprinter
sprinter

Reputation: 27986

You might consider converting your strings to an enum:

public enum Status {
    ACTIVE("active", WORKING),
    INACTIVE("inactive", NOT_WORKING),
    IN_SERVICE("inservice", SERVICE),
    ...

    private final String name;
    private final Value value;

    Status(String name, Value value) {
        this.name = name;
        this.value = value;
    }

    public static Optional<Value> getValueForName(String name) {
        return Arrays.stream(values())
            .filter(v -> isEqual(name, this.name))
            .findAny();
    }
}

Upvotes: 0

Seelenvirtuose
Seelenvirtuose

Reputation: 20658

The OOP approach is to have this method in your interface IValue2:

interface IValue2 {
    ...
    String getName();
    IValue1 mapValue();
}

Each implementing object now must override this abstract method. This maybe requires some implementation changes. You could - for example - have a ActiveValue2 class:

class ActiveValue2 implements IValue2 {
    ...
    public String getName() {
        return StatusValues.ACTIVE;
    }
    public IValue1 mapValue() {
        return ValueHelper.getName(StatusValues2.WORKING);
    }
}

You now simply call the mapValue method on an IValue2-types variable. Done.

Upvotes: 2

Daniel Hintze
Daniel Hintze

Reputation: 477

A map would do fine I guess. You could also think about switch-case to make it a little less verbose:

private IValue1 mapValue(IValue2 value2) {
 switch(value2.getName()) {
     case StatusValues.ACTIVE:     return StatusValues2.WORKING;
     case StatusValues.INACTIVE:   return StatusValues2.NOT_WORKING;
     case StatusValues.IN_SERVICE: return StatusValues2.SERVICE;
     default: throw new RuntimeException();
 }

Upvotes: 2

Kesem David
Kesem David

Reputation: 2245

Yes, having a Map of <StatusValues, StatusValues2> would be the right approach.

You will simply look for the current StatusValue as the key and receive the matching StatusValue2

Upvotes: 0

Related Questions