2
\$\begingroup\$

I tackled this problem today and was wondering how I could possibly improve this code. The objective of the method is to return an array with the respective scores of each player. The method takes three values from 2 users. a0,a1,a2 are all scores from player a. b0,b1,b2 are all scores of player b.

A player is awarded a point if their score of the same category is bigger than the score of the player. For example to determine who got a better first score we must compare a0 with b0 and so on forth.

Here is my code:

static int[] solve(int a0, int a1, int a2, int b0, int b1, int b2){
        int p1 = 0;
        int p2 = 0;
        if(a0 > b0){
            p1++;
        }else if(a0 < b0){
            p2++;
        }

        if(a1 > b1){
            p1++;
        }else if(a1 < b1){
            p2++;
        }

        if(a2 > b2){
            p1++;
        }else if(a2 < b2){
            p2++;
        }

        return new int[]{p1,p2};

    }
\$\endgroup\$
  • \$\begingroup\$ Could a0, a1, a2 be an array instead of three separate arguments? \$\endgroup\$ – JS1 Jul 3 '17 at 9:20
  • \$\begingroup\$ @JS1 This is how the problem was given to me. \$\endgroup\$ – TheLearner Jul 3 '17 at 9:27
  • \$\begingroup\$ In that case what you have written seems fine. \$\endgroup\$ – JS1 Jul 3 '17 at 9:28
2
\$\begingroup\$

As Timothy mentioned, your naming of the variables are not so good. I would recommend you have a look at Java Naming Conventions. It will be helpful for writing easily readable codes.

Secondly, try using private for your method. I know it is not a big program, but you will get used to it.

Thirdly, the beauty of the code is in shortness and effectiveness. And here I took Timothy's code and wrote it with for loop:

int[] scoresOfPlayerA = new int[]{a0,a1,a2};
int[] scoresOfPlayerB = new int[]{b0,b1,b2};

for (int i = 0; i < scoresOfPlayerA.length; i++){
    if (scoresOfPlayerA[i] > scoresOfPlayerB[i]){
        finalScoreOfA++;
    } else if (scoresOfPlayerA[i] < scoresOfPlayerB[i]) {
        finalScoreOfB++;
    }
}

return new int[]{finalScoreOfA, finalScoreOfB};

Good luck with your future codes.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Thanks for sharing the code!

The biggest concern here (beside naming) is don't repeat yourself!.

You have almost the same code three times:

    if(a0 > b0){
        p1++;
    }else if(a0 < b0){
        p2++;
    }

    if(a1 > b1){
        p1++;
    }else if(a1 < b1){
        p2++;
    }

    if(a2 > b2){
        p1++;
    }else if(a2 < b2){
        p2++;
    }

The trick is: turn this similar code into equal code.

You could do so like this:

The scores to compare could also be organized as arrays as you already do for the output:

    int[] scoresPlayerA = new int[]{a0,a1,a2};
    int[] scoresPlayerB = new int[]{b0,b1,b2};
    if(scoresPlayerA[0]  >scoresPlayerB[0]  ){
        p1++;
    }else if(scoresPlayerA[0] <scoresPlayerB[0]) {
        p2++;
    }
    if(scoresPlayerA[1]  >scoresPlayerB[1]  ){
        p1++;
    }else if(scoresPlayerA[1] <scoresPlayerB[1) {
        p2++;
    }        
    if(scoresPlayerA[2]  >scoresPlayerB[2]  ){
        p1++;
    }else if(scoresPlayerA[2] <scoresPlayerB[2) {
        p2++;
    }
    return new int[]{p1,p2};

Now the only difference in that 2 parts is the index in the scoresPlayer... arrays. This is best replaced with a for loop...

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

To add another answer, if you use Java you could do more OO. I'm not sure if it fits your domain and requirements, but if you want to know if Player A's score is better than Player B's, you could do this:

playerA.compareScoreTo( playerB );

Which returns how many points A has in advantage to B. This is similar how comparators work in Java.

With class Player:

public class Player
{
    int scores[];

    public int compareScoreTo( Player other )
    {
        int p1Score = 0;
        int p2Score = 0;

        for ( int i = 0; i < scores.length; i++ )
        {
            if ( scores[i] > other.scores[i] )
            {
                p1Score++;
            }
            else if (scores[i] < other.scores[i] ){
                p2Score++;
            }

        }
        return p1Score - p2Score;
    }

    ....
}
| improve this answer | |
\$\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.