Making if-else solution more efficient, less lines of code (Java)

Question

Is there a more efficient way to code something like this without using as many if-else statements?

private int group1, group2, group3, group4;
private int total = 0

public void assignMembers()
{
    group1 = (int)((6 * Math.random()) + 1);
    group2 = (int)((6 * Math.random()) + 1);
    group3 = (int)((6 * Math.random()) + 1);
    group4 = (int)((6 * Math.random()) + 1);
}

public void calculateSomething()
{
    if(group1 == 3)
    {
        total += 2;
    }
    else if(group1 == 5)
    {
        total += 4;
    }

    if(group2 == 3)
    {
        total += 2;
    }
    else if(group2 == 5)
    {
        total += 4;
    }

    if(group3 == 3)
    {
        total += 2;
    }
    else if(group3 == 5)
    {
        total += 4;
    }

    if(group4 == 3)
    {
        total += 2;
    }
    else if(group4 == 5)
    {
        total += 4;
    }
{

The if-else statements are adding two to the total if the group has 3 member and 4 if the group has 5 members.

I know I can maybe do something more efficient with a "groups" array, but is there a way without an array? Maybe a way for the calculateSomething method to get the number of team member of each group without having to repeat if-else so much? Any suggestions would be appreciated.

You should also consider putting all your group variables into an Array (or a list). This makes it easier to do things to all of them by iterating over the data structure, and makes it easier and more efficient to store a lot of variables, if you need more than 4. — MercyBeaucou
– MercyBeaucou, Commented Oct 8, 2016 at 3:34

Enzokie · Accepted Answer · 2016-10-09 00:19:14Z

4

If you seem find a redundant pattern in your code that's the time you are going to create a re-usable function.

private int group1, group2, group3, group4;
private int total = 0;

    public void assignMembers()
    {
        group1 = (int)(Math.random()*6 + 1);
        group2 = (int)(Math.random()*6 + 1);
        group3 = (int)(Math.random()*6 + 1);
        group4 = (int)(Math.random()*6 + 1);

        calc(group1);
        calc(group2);
        calc(group3);
        calc(group4);
    }

    public void calc(int group)
    {
        switch (group){
                case 3:
                  total += 2;
                  break;
                case 5:
                  total += 4;
                  break;
        }
    }

Update answer - since the requirements is : The method must be called outside the class.

private int group1, group2, group3, group4;
    private int total = 0;

        public void assignMembers()
        {
            group1 = (int)(Math.random()*6 + 1);
            group2 = (int)(Math.random()*6 + 1);
            group3 = (int)(Math.random()*6 + 1);
            group4 = (int)(Math.random()*6 + 1);
        }

        private void calc(int group)
        {
            switch (group){
                    case 3:
                      total += 2;
                      break;
                    case 5:
                      total += 4;
                      break;
            }
        }

        public void calculateSomething(){
            calc(group1);
            calc(group2);
            calc(group3);
            calc(group4);
        }

edited Oct 9, 2016 at 0:19

answered Oct 8, 2016 at 2:58

Enzokie

7,4156 gold badges35 silver badges42 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Bluasul Over a year ago

Great answer, thank you! What if the calc() method were being called from another class which can't be modified and it has no parameters? Would having calc() in that class and calc(int group) in this class create problems?

Enzokie Over a year ago

@Bluasul I updated my answer based on your updated requirements. Now you can call it outside the class.

Kasun Siyambalapitiya · Accepted Answer · 2016-10-08 03:26:48Z

Since you have a redundant pattern in your code

    private int group1, group2, group3, group4;
    private int total = 0;

    public void assignMembers()
    {
        group1 = randomGen();
        group2 = randomGen();
        group3 = randomGen();
        group4 = randomGen();

        function(group1);
        function(group2);
        function(group3);
        function(group4);
    }

    public int randomGen(){
        int x=(int)(Math.random()*6 + 1);
        return x;
    }
    public void function(int group)
    {
        switch (group){
                case 3:
                  total += 2;
                  break;
                case 5:
                  total += 4;
                  break;
                default:
                  // write here what you need to perform when the group value is 3 or 5

        }
    }

for more info visit this site

tenshiman · Accepted Answer · 2016-10-08 03:27:29Z

1

assuming that you're writing java, you should write a case statement and pass each variable to the function. you should define total in the first function as well but i won't show you how. anyway something like this then pass each group to it in a for loop:

public int calculateSomething(groupx){
    switch (groupx) 
        {
            case 3:
            total += 2;
            break;
            case 5:
            total += 4;
            break;
        }

note that case doesn't need brackets around the proceeding line.

edited Oct 8, 2016 at 3:27

answered Oct 8, 2016 at 2:47

tenshiman

564 bronze badges

3 Comments

user4910279 Over a year ago

total is incremeted by 6, when groupx is 3.

tenshiman Over a year ago

no its not? or do you mean it should be different for group3? in that case you need to write a new func with a sig like calculateSomething2(groupx) and make the 5th line be "=+6" instead of 3

Adrian Colomitchi Over a year ago

@tenshiman You may want to refresh your knowledge of switch/case statement, especially what a break does (and what happens if missing).

Bohemian · Accepted Answer · 2016-10-08 16:16:13Z

Prefer a "data" approach over a "code" one for what is a data-centric problem.

First, define the extra points declaratively.

private static Map<Integer, Integer> extras = new HashMap<Integer, Integer>() {{
    put(3, 2);
    put(5, 4);
}};

Notice that this is the only place in the code that these numbers appear, and that changing them or adding more is simple and it's obvious how to do it.

Then use a stream to process all groups in one line:

public void calculateSomething() {
    total += IntStream.of(group1, group2, group3, group4)
      .map(i -> extras.getOrDefault(i, 0))
      .sum();
}

Using the map avoids even one if and the code simple and automatically reused by the stream.

_{Disclaimer: Code may not compile or work as it was thumbed in on my phone (but there's a reasonable chance it will work)}

user4910279 · Accepted Answer · 2016-10-08 03:31:17Z

0

Try this.

private int group1, group2, group3, group4;
private int total = 0;

public void assignMembers() {
    group1 = updateTotal((int) ((6 * Math.random()) + 1));
    group2 = updateTotal((int) ((6 * Math.random()) + 1));
    group3 = updateTotal((int) ((6 * Math.random()) + 1));
    group4 = updateTotal((int) ((6 * Math.random()) + 1));
}

int updateTotal(int group)
{
    total += group == 3 ? 2 : group == 5 ? 4 : 0;
    return group;
}

answered Oct 8, 2016 at 3:31

user4910279

Collectives™ on Stack Overflow

Making if-else solution more efficient, less lines of code (Java)

5 Answers 5

2 Comments

Comments

3 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

Comments

3 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related