And the little one said "Roll over"

Question

Below is some code I have written today to test out some bits I have learnt. It isn't much nor is it spectacular. Please critique and let me know what I could/should have done differently or anything I could do to improve it so far.

package easy8;

import java.util.Scanner;

public class song99bottles {

public static void main(String[] args) {

// Declare a reference variable of type song99bottles - new object  
song99bottles go = new song99bottles();
// calls the method "queston" for object "go".
go.question();

}

public void question(){

    song99bottles start = new song99bottles();

    String answer;

    System.out.println("Would you like to hear a nursery rhyme?\nPlease enter yes or no:");
    Scanner input = new Scanner(System.in);
    answer = input.next();
    if(answer.equalsIgnoreCase("yes")){
        start.lyrics();
    } else{
        System.out.println("Bye");
    }

}



public void lyrics() {

    int peeps  = 10;
    String intro = "There were ";
    String intro2 = " in the bed and the little one said rollover";
    String fall = "\nSo they all rolled over and 1 fell out.";
    String end = "And the little said 'I'm lonely'.";

    while (peeps > 0){
        System.out.println( intro + peeps + intro2 + fall);
        peeps--;
        if (peeps == 1){
            System.out.println(end);
            return;
                        }
    }


}

}

Answer 1

1. Class names should be PascalCase (song99bottles -> Song99Bottles)

2. You should indent your code or you will end up with a long code and you don't understand the various { } pairs (and an horrible code!)

   public class song99bottles {
   public static void main(String[] args) {
   // Declare a reference variable of type song99bottles - new object  
   song99bottles go = new song99bottles();
   // calls the method "queston" for object "go".
   go.question();   
   }
   // [...]
   

   if you want to follow Code conventions, you code should look like this

   public class Song99Bottles {
   
       public static void main(String[] args) {
           // Declare a reference variable of type song99bottles - new object
           Song99Bottles go = new Song99Bottles();
           // calls the method "queston" for object "go".
           go.question();
       }
   
       // [...]
   

   Personally i follow the new-line style, but you will see with time you will get "your style".

   i hope you understand the concept. Read wikipedia page.

3. You should give to your variables a more descriptive name, you should understand the scope of the variable by just reading the name. (song99bottles go = new song99bottles(); go? What is? a countdown? I could call it song (example))

---

About your code:

1. You create two song99bottles objects, why? One inside main and one inside question(). You should move question code inside main because song99bottles class should just play the song, the main will take care if the user want or not to play the song. Then in your main, just call go.lyrics();

2. About lyrics() code, see Roger answer.

Answer 2

Taking Roger's answer as a starting point, you can also only use one string in the lyrics() function, because you always use intro2 before fall. If you use one string, you save an allocation and concatenation.

String intro = "There were ";
String fall  = " in the bed and the little one said rollover"
               + System.lineSeparator()
               + "So they all rolled over and 1 fell out.";
String end   = "And the little said 'I'm lonely'.";

for(int peeps = 10; peeps > 0; peeps--){
    System.out.println(intro + peeps + fall);
}
System.out.println(end);

Answer 3

There are a number of improvements you can make, but removing the declaration for peeps in lyrics() and changing the while loop to a for loop would be a good start:

for(int peeps=10;peeps>0;peeps--) {
    System.out.println( intro + peeps + intro2 + fall);            
}
System.out.println(end);

Answer 4

Suggestion to code

1. Do not create Song99bottles twice, from question() you can call lyrics()
2. Do not + strings, it creates an unbelievable overhead of strings in the stack (memory). Use a string formatter String.format("format %d", integer)
3. Do not use comments unless the really matter. Comments are a sign of bad names and too few funtions, see this http://ptgmedia.pearsoncmg.com/images/9780132350884/samplepages/0132350882_Sample.pdf

Answer 5

In addition to the other good advice I see here, I would rename your lyrics method to imply exactly what it does. For example printLyrics().

Answer 6

The only thing I can think to add to others' is that the comments in the main method are superfluous.

// Declare a reference variable of type song99bottles - new object  
song99bottles go = new song99bottles();

It's obvious you're declaring a reference here. You're not adding much by saying it.

// calls the method "queston" for object "go".
go.question();

This is worse. This is like saying x = 10; // Sets x to 10. If you're set on leaving some comments it'd be more beneficial to leave some in the other methods with more logic involved like the lyrics method.