Camp challenges in one program

Question

My counselor in a camp I went to over the summer gave me a code challenge. To take 6 challenges (listed below) and have all of them in one program, and have the user choose which one to use. Please review all of my code. If there is anything wrong, inefficient, or if I use bad practices, let me know; I'm still new at this. PS: If there are any methods I made that are already built into Java, ignore them, I had to reinvent the wheel

1. Given a list of numbers, add up all the even numbers from that list and return the sum.

2. Given a list of numbers, return whether that list is in order or not

3. Given two words, return a list of letters that occur in both words

4. Given a long string representing a sentence, and a shorter string representing a word, find if the word is in the sentence.

5. Given a string return which letter is most frequent.

6. Given a string, make it a palindrome. You may only add characters to the end of the string.

package Challenges;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Scanner;
import java.util.Set;

public class ChallengeOneToFive {
@SuppressWarnings("resource")
public static void main(String[] args) {
    System.out.println("Which Challenge would you like to see?\n1: Even Number Adder \n2: Number Sorter \n3: Word Letter Occurance \n4: Sentence Words \n5: Frequent Letters \n6: Palindromes");

    Scanner scan = new Scanner(System.in);

    String choice = scan.nextLine();

    if(choice.equals("1")) {
        Challenge1();
    } else if(choice.equals("2")) {
        Challenge2();
    } else if(choice.equals("3")) {
        Challenge3();
    } else if(choice.equals("4")) {
        Challenge4();
    } else if(choice.equals("5")) {
        Challenge5();
    } else if(choice.equals("6")) {
        Challenge6();
    }
}
@SuppressWarnings("resource")
//Challenge 1 is Completed

public static int Challenge1() {
    System.out.println("How many numbers will you be entering?");
    Scanner scan = new Scanner(System.in);
    int numberOfNumbers = scan.nextInt();
    int sum = 0;
    for(int i = 0; i < numberOfNumbers; i++) {
        System.out.println("Enter a number");
        int number = scan.nextInt();
        if(number % 2 == 0) {
            sum+=number;
        }
    }
    System.out.println("The Sum is " + sum);
    return sum;
}
@SuppressWarnings({ "resource" })
public static boolean Challenge2() {
    boolean inOrder = false;
    System.out.println("How many numbers will you be entering?");
    Scanner scan = new Scanner(System.in);
    int numberOfNumbers = scan.nextInt();
    int[] numbers = new int[numberOfNumbers];

    for(int i = 0; i < numberOfNumbers; i++) {
        System.out.println("Enter a number");
        int number = scan.nextInt();
        numbers[i] = number;
        if(i != 0){
            if((numbers[i] - numbers[i - 1]) > 0) {
                inOrder = true;
            } 
            if((numbers[i] - numbers[i - 1]) < 0){
                inOrder = false;
            }
        } else if(i == 0){
                numbers[0] = 0;
        }
    }
    System.out.println("The fact that your numbers are in order is " + inOrder);
    return inOrder;


}

//Challenge 3 is Completed
@SuppressWarnings({ "resource", "unused" })
public static String Challenge3() {
    System.out.println("Enter a String");
    Scanner scan = new Scanner(System.in);
    String word1 = scan.nextLine();
    System.out.println("Enter another String");
    String word2 = scan.nextLine();
    String list = "";
    System.out.println("");
    Character[] word1Chars = toCharacterArray(word1);
    Character[] word2Chars = toCharacterArray(word2);
    Set<String> s1 = new HashSet<>( Arrays.asList( word1.split( "" ) ) );
    Set<String> s2 = new HashSet<>( Arrays.asList( word2.split( "" ) ) );
    s1.retainAll( s2 );

    for( String c: s1 ){
        System.out.print(c + " ");
    }
    return list;
}

public static boolean Challenge4() {
    boolean isInSentence = false;
    System.out.println("Enter a sentence");
    Scanner scan = new Scanner(System.in);
    String sentence = scan.nextLine();
    System.out.println("Now enter a word");
    String word = scan.nextLine();
    scan.close();
    if(sentence.contains(word)) {
        isInSentence = true;
    }
    if(isInSentence == true) {
        System.out.println("The word you entered is in the string");
    } else {
        System.out.println("The word you entered is not in the string");
    }

    return isInSentence;
}

//Challenge 5 is Completed
@SuppressWarnings("resource")
public static char Challenge5() {
    int characterCount = 0;
    int maxCharacter = 0;
    char maxCharacterChar = '.';
    System.out.println("Enter a string");
    Scanner scan = new Scanner(System.in);
    String word = scan.nextLine();
    Character[] cArray = toCharacterArray(word);

    for(int i =0; i < cArray.length; i++)
    {
        int characterASCII = (int)cArray[i];
        characterCount = 0;


        for(int x = 0; x < cArray.length; x++)
        {
            if(characterASCII == (int)cArray[x])
            {
                characterCount ++;
                //System.out.print("Character Count for " + characterASCII + " " +  characterCount  + "\n");
                if(characterCount > maxCharacter)
                {
                    maxCharacter = characterCount;
                    maxCharacterChar = cArray[i];
                }
            }
        }
    }
    System.out.println("The character that occurs the most is " + maxCharacterChar);
    return maxCharacterChar;
}

 @SuppressWarnings("resource")
public static String Challenge6() {
     System.out.println("Enter a string to be palindrome'd");
     Scanner scan = new Scanner(System.in);
     String palindrome = scan.nextLine();
     StringBuilder sb = new StringBuilder(palindrome);
     String sb2 = sb.toString();
     Character[] charArray = toCharacterArray(sb2);
     Character[] reverse = new Character[charArray.length];
     for(int o = 0; o < sb2.length(); o++) {
         reverse[o] = sb2.charAt(o);
     }

     for(int i = reverse.length - 1; i > -1; i--) {
            sb2 = sb2 + reverse[i];
     }
     System.out.println(sb2);


    return sb2;
}


 public static Character[] toCharacterArray( String s ) {

       if ( s == null ) {
         return null;
       }

       int len = s.length();
       Character[] array = new Character[len];
       for (int i = 0; i < len ; i++) {
          array[i] = new Character(s.charAt(i));
       }

       return array;
    }
}

Answer 1

@SuppressWarnings("resource")

That is a bad sign. That probably means you didn't close your scanner; close it like so:

scan.close();

---

@SuppressWarnings({ "resource", "unused" })

How many warnings do you have? ANY of them means bad code. In this case, some of your variables are unused. Remove them.

 public static Character[] toCharacterArray( String s ) {

       if ( s == null ) {
         return null;
       }

       int len = s.length();
       Character[] array = new Character[len];
       for (int i = 0; i < len ; i++) {
          array[i] = new Character(s.charAt(i));
       }

       return array;
    }
}

Don't reinvent the wheel, unless you were required to. Java has a String.toCharArray() method that you can use easily, e.g.:

 Character[] charArray = toCharacterArray(sb2);

to

Character[] charArray = sb2.toCharArray();

Your method names are currently in PascalCase, when they should be camelCase. Challenge1 should be challenge1, Challenge2 should be challenge2, etc.

Now to individual challenges:

Challenge 1:

No problems there, just some minor readability improvements (plus closing scanner):

public static int challenge1() {
    System.out.println("How many numbers will you be entering?");
    Scanner scan = new Scanner(System.in);
    int numberOfNumbers = scan.nextInt();
    int sum = 0;
    for (int i = 0; i < numberOfNumbers; i++) {
        System.out.println("Enter a number");
        int number = scan.nextInt();
        if (number % 2 == 0) {
            sum += number;
        }
    }
    System.out.println("The Sum is " + sum);
    scan.close();
    return sum;
}

Challenge 2:

I would do a complete rewrite of that. You don't need an array, and you don't need that complex of code. Here is how I would do it:

public static boolean challenge2() {
    System.out.println("How many numbers will you be entering?");
    Scanner scan = new Scanner(System.in);
    int numberOfNumbers = scan.nextInt();

    for (int i = 0, previousNumber = Integer.MIN_VALUE; i < numberOfNumbers; i++) {
        System.out.println("Enter a number");
        int number = scan.nextInt();
        if (number > previousNumber) {
            // out of order
            System.out.println("The fact that your numbers are in order is false");
            scan.close();
            return false;
        }
    }
    System.out.println("The fact that your numbers are in order is true");
    scan.close();
    return true;

}

Challenge 3:

I don't quite understand challenge 3...

From the new comments added and after looking at your code, I have come up with...

Some things to note:

• System.out.println(""); can just be System.out.println();
• Some variables you don't use
  • e.g. list, which you return as empty
  • the char arrays
• You used Sets: good!
  • You use Sets properly: good!

After some (minor) changes:

public static Set<Character> challenge3() {
    System.out.println("Enter a String");
    Scanner scan = new Scanner(System.in);
    String word1 = scan.nextLine();
    System.out.println("Enter another String");
    String word2 = scan.nextLine();
    scan.close();
    Set<Character> set = new HashSet<>(Arrays.asList(word1.toCharArray()));
    set.retainAll(new HashSet<>(Arrays.asList(word2.toCharArray())));
    System.out.println();
    for (char c : set) {
        System.out.print(c + " ");
    }
    return set;
}

Challenge 4:

Well, easy. You are over-complicating it. Why:

1. Two if statements are not necessary. It can easily be one.
2. Well, that's it really.

After simplifications:

public static boolean challenge4() {
    System.out.println("Enter a sentence");
    Scanner scan = new Scanner(System.in);
    String sentence = scan.nextLine();
    System.out.println("Now enter a word");
    String word = scan.nextLine();
    scan.close();
    if (sentence.contains(word)) {
        System.out.println("The word you entered is in the string");
        return true;
    }
    System.out.println("The word you entered is not in the string");
    return false;
}

Challenge 5:

I'll come back to this.

This challenge is a bit more tricky. Your solution is again too complex:

• x shouldn't start a 0, but at i. This will improve performance. (or at least this is the case for my solution)

• The two nested if statements can be simplified to one:

      if (characterASCII == (int)cArray[x] && ++characterCount > maxCharacter) {
          maxCharacter = characterCount;
          maxCharacterChar = cArray[i];
      }
  

The solution I would use:

1. Loop through the string:
   • If the character has not been counted, count the number of occurrences of that character in the string

Seems simple enough; let's try implementing it:

public static char challenge5() {
    System.out.println("Enter a string");
    Scanner scan = new Scanner(System.in);
    String word = scan.nextLine();
    scan.close();
    char[] chars = word.toCharArray();
    int[] numberOfOccurences = new int[256]; // Number of ASCII characters possible
    char result;
    int maxCount = 0;
    for (int i = 0; i < array.length; i++) {
        int index = (int) chars[i];
        if (numberOfOccurrences[index] == 0) { // not counted yet
            numberOfOccurrences[index] = countOccurrences(chars, chars[i], i);
            if (numberOfOccurrences[index] > maxCount) {
                maxCount = numberOfOccurrences[index];
                result = chars[i];
            }
        }
    }
    System.out.println("The character that occurs the most is " + result);
    return result;
}

private static int countOccurrences(char[] array, char c, int startIndex) {
    int count = 0;
    for (int i = startIndex; i < array.length; i++) {
        if (array[i] == c) {
            count++;
        }
    }
    return count;
}

Challenge 6:

StringBuilder has a reverse() method; again, don't reinvent the wheel:

public static String challenge6() {
     System.out.println("Enter a string to be palindrome'd");
     Scanner scan = new Scanner(System.in);
     String palindrome = scan.nextLine();
     scan.close();
     return new StringBuilder(palindrome).reverse().toString();
}

Answer 2

Your menu could be replaced by a switch instead. Switches are more readable, faster, easier to maintain/expand and the idiomatic way of doing such things:

    int choice = Integer.valueOf(scan.nextLine());
    switch (choice) {
        case 1:  Challenge1();
                 break;
        case 2:  Challenge2();
                 break;
        case 3:  Challenge3();
                 break;
        case 4:  Challenge4();
                 break;
        case 5:  Challenge5();
                 break;
        case 6:  Challenge6();
                 break;
        default: System.out.println("Invalid challenge selected");
                 break;

You may want a construction that keeps asking the user for valid input unless he enters a given exit sequence.

Challenge 1 and 2 can handle different amounts of input. While it's not required for challenge 3, it would clean-up your code considerably. The moment you start using variable names like word2 and s2 you should ask yourself whether those are descriptive. Descriptive variable naming helps a lot when adding features later on or trying to understand what you've written a long time ago when looking back on it.

Answer 3

Bugs in challenge 2

Challenge 2 (whether a list of numbers is in order) is not correct. You currently are printing true if the last two numbers are in order, not if the whole list is in order. Furthermore, you have some strange piece of code that sets the first number to zero, for no particular reason.

To fix the first problem, you should start inOrder as true, and then when you see any out-of-order pair of numbers, you can set inOrder to false. You should not set inOrder to true other than before the loop.

To fix the second problem, just don't set the first number to zero (I'm not sure why you did that to begin with).

Possible overflow

Another problem is that this if statement:

        if((numbers[i] - numbers[i - 1]) < 0){
            inOrder = false;
        }

should be written like this:

        if (numbers[i] < numbers[i - 1]) {
            inOrder = false;
        }

You may think that they mean the same thing, but the first version can lead to integer overflow. For example, if numbers[i] were -2000000000 and numbers[i-1] were 2000000000, the if statement would evaluate to false when it should actually be true.

Fixed version

Here is a rewrite of your function with all of the above problems fixed:

public static boolean Challenge2() {
    boolean inOrder = true;
    System.out.println("How many numbers will you be entering?");
    Scanner scan = new Scanner(System.in);
    int numberOfNumbers = scan.nextInt();
    int[] numbers = new int[numberOfNumbers];

    for(int i = 0; i < numberOfNumbers; i++) {
        System.out.println("Enter a number");
        numbers[i] = scan.nextInt();
        if (i != 0 && numbers[i] < numbers[i - 1]) {
            inOrder = false;
        }
    }
    System.out.println("The fact that your numbers are in order is " + inOrder);
    return inOrder;
}

Answer 4

If it is Java 7 you could change:

Scanner scan = new Scanner(System.in);

String choice = scan.nextLine();
/*...*/    

}

into

try(Scanner scan = new Scanner(System.in)){ 
    String choice = scan.nextLine();        
    /*...*/
}

then it will autoclose the resource and you can get rid of suppress warnings

Answer 5

You are opening a Scanner on the same stream multiple times. This can create odd results because each scanner is separate and each can try to read ahead.

Instead create it once and pass it to each of the challenges:

if(choice.equals("1")) {
    Challenge1(scan);
} else //...

This means that you don't need to create and close the scanner in each of the challenges:

public static int challenge1(Scanner scan) {
    System.out.println("How many numbers will you be entering?");

    int numberOfNumbers = scan.nextInt();
    int sum = 0;
    for (int i = 0; i < numberOfNumbers; i++) {
        System.out.println("Enter a number");
        int number = scan.nextInt();
        if (number % 2 == 0) {
            sum += number;
        }
    }
    System.out.println("The Sum is " + sum);

    return sum;
}