current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

Please can someone advise me, how I could use a loop for this code, in the interests of code reuse.

My code is as follows

public class Clubs{

    public void printGreeting(int choice, String clubName[]) {


       System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


        if (choice==1){
            System.out.println("You chose: " + clubName[0]);
        }
        if (choice==2){
            System.out.println("You chose: " + clubName[1]);
        }
        if (choice==3){
            System.out.println("You chose: " + clubName[2]);
        }
        if (choice==4){
            System.out.println("You chose: " + clubName[3]);
        }
        if (choice==5){
            System.out.println("You chose: " + clubName[4]);
        }
        if (choice==6){
            System.out.println("You chose: " + clubName[5]);
        }
        if (choice==7){
            System.out.println("You chose: " + clubName[6]);
        }
        if (choice==8){
            System.out.println("You chose: " + clubName[7]);
        }
        if (choice==9){
            System.out.println("You chose: " + clubName[8]);
        }
        if (choice==10){
            System.out.println("You chose: " + clubName[9]);
        }
        if (choice==11){
            System.out.println("You chose: " + clubName[10]);
        }
        if (choice==12){
            System.out.println("You chose: " + clubName[11]);
        }
        if (choice==13){
            System.out.println("You chose: " + clubName[12]);
        }
        if (choice==14){
            System.out.println("You chose: " + clubName[13]);
        }
        if (choice==15){
            System.out.println("You chose: " + clubName[14]);
        }
        if (choice==16){
            System.out.println("You chose: " + clubName[15]);
        }
        if (choice==17){
            System.out.println("You chose: " + clubName[16]);
        }
        if (choice==18){
            System.out.println("You chose: " + clubName[17]);
        }
        if (choice==19){
            System.out.println("You chose: " + clubName[18]);
        }
        if (choice==20){
            System.out.println("You chose: " + clubName[19]);
        }
    }
}
share|improve this question
1  
What is clubName? Full code is more welcome than partial –  Caridorc yesterday
2  
Couldn't you just do : System.out.println("You chose: " + clubName[choice - 1]); ? –  TopinFrassi yesterday

2 Answers 2

up vote 10 down vote

A minor thing first, in Java it is recommended to use String[] clubName rather than String clubName[]. It will have the same effect, but writing String[] is clearer.

You ask for a loop, so I'll try to explain how to reach the conclusion...

Regular, normal, for loops looks like this:

for (int i = START; i < MAX; i++) {
    // code
}

In this case, as your if-statements for choice goes from 1 to 20, START will be 1 and MAX will be 20. However, we need to use <= as we want to loop until 20 inclusive.

for (int i = 1; i <= 20; i++) {
    // code
}

So, what about the // code part? Clearly it should be something like this:

if (choice == XXX) {
    System.out.println("You chose: " + clubName[YYY]);
}

As the XXX and YYY parts are the only things that change in your code. Now, what exactly should we put at XXX and YYY?

XXX goes from 1 to 20, just like our i variable in the for-loop. So that should be i, but what about YYY? That is always one less than i? Should we have another for-loop to loop from 0 to 19? No! We should use the same i variable, we can just reduce one from it, so we get this:

for (int i = 1; i <= 20; i++) {
    if (choice == i) {
        System.out.println("You chose: " + clubName[i - 1]);
    }
}

However, as that condition is only true once (choice cannot be both 4 and 7 at the same time for example), looping through all the numbers from 1 to 20 is a bit unnecessary. Instead, we can do this:

if (choice >= 1 && choice <= 20) {
    System.out.println("You chose: " + clubName[choice - 1]);
}

We check if the choice value is between 1 and 20 (inclusive), and if it is we print out the corresponding index in clubName, which we get by taking choice - 1.

share|improve this answer
    
@Justin yes, fixed. –  Simon Forsberg♦ yesterday
up vote 5 down vote

I recommend using subtraction and array indexing to solve your problem.

System.out.println("You chose: " + clubName[choice - 1])
share|improve this answer
2  
This fails if choice is outside 1 - 20. –  Simon Forsberg♦ yesterday
    
@SimonForsberg it does, an if to check for boundaries may be added if needed –  Caridorc yesterday
    
I came in to add exactly this. –  Legato yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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