alxmke
alxmke

Reputation: 334

Java Program Optimization Involving String Comparison

So, I made this simple program which tests two string values (representative of two individual primary colours) for difference and type in order to determine the colour of the resultant mixture.

/**
 * A program that prompts the user to enter the names of two different primary colors to create a mixture.
 * 
 * @author A. Mackey
 * @version 1.0
 */
import java.util.*;
public class ColourMixer {
    public static void main(String [] args) {
        colourMixer();
    }

    public static void colourMixer() {
        Scanner in = new Scanner(System.in);
        String colourOne = "";
        String colourTwo = "";

        System.out.println("You are mixing two different primary colours.");

        System.out.print("Enter your first colour: ");
        colourOne = in.nextLine();

        System.out.print("Enter your second colour: ");
        colourTwo = in.nextLine();

        if((colourOne.equalsIgnoreCase("red") || colourOne.equalsIgnoreCase("blue") ) && (colourTwo.equalsIgnoreCase("blue") || colourTwo.equalsIgnoreCase("red") && !(colourOne.equalsIgnoreCase(colourTwo)))) {
            System.out.println("Your colour combination creates purple!");
        } else if ((colourOne.equalsIgnoreCase("red") || colourOne.equalsIgnoreCase("yellow") ) && (colourTwo.equalsIgnoreCase("yellow") || colourTwo.equalsIgnoreCase("red") && !(colourOne.equalsIgnoreCase(colourTwo)))) {
            System.out.println("Your colour combination creates orange!");
        } else if ((colourOne.equalsIgnoreCase("blue") || colourOne.equalsIgnoreCase("yellow") ) && (colourTwo.equalsIgnoreCase("yellow") || colourTwo.equalsIgnoreCase("blue") && !(colourOne.equalsIgnoreCase(colourTwo)))) {
            System.out.println("Your colour combination creates green!");
        } else
            System.out.println("You have not entered two different primary colours.");
    }
}

It functions exactly as intended, with no observable issues.

However, it feels clunky and excessive in its logic.

I'm curious if this algorithm could be improved.

Upvotes: 0

Views: 116

Answers (4)

user
user

Reputation: 745

You could put the input Strings (in my case AS lowercase string ) into a Set and could call Sets contains.

If setContains(red) --> check if contains yellow or blue. Now you can choose seither setContains(blue) or setContains(yellow) and test if one of the other colours is present because by doing it this way the combination of the colour doesnt matter and all possible cases are tested. yellow and blue will be the same as blue and yellow.

Set<String> set = new HashSet<String>();
set.add(colourOne.toLowerCase());
set.add(colourTwo.toLowerCase());

if(set.contains("red")){
    if(set.contains("blue")){
        //return purple
    }
    if(set.contains("yellow"){
        //returnorange
    }
}
if(set.contains("yellow")){
    if(set.contains("blue"){
        //return green
    }
}
//unidentified colour

Upvotes: 0

Daniel
Daniel

Reputation: 1861

This looks like the perfect place to use enums.

enum COLOR{
  RED,GREEN,BLUE,BLACK,WHITE; //etc.
}

And then you could use a method to convert from user input string to colors:

public static final COLOR convertToColor(final String input){
    try{
        return COLOR.valueOf(input.toUpperCase());
    }catch(IllegalArgumentException e){
       //invalid color, handle it properly
       System.out.println(input + " is not a valid color !! Enter a new valid color this time!!");
       return null;
    }
}

public static void colourMixer() {
    Scanner in = new Scanner(System.in);
    COLOR color1;
    COLOR color2;

    System.out.print("Enter your first colour: ");
    do{
       color1 = convertToColor(in.nextLine());
    }while( color1 == null);

    System.out.print("Enter your second colour: ");
    do{
       color2 = convertToColor(in.nextLine());
    }while( color2 == null);

    if(COLOR.RED.equals(color1) && COLOR.BLUE.equals(color2)){
          System.out.println("Your colour combination creates purple!");
    }else if(COLOR.RED.equals(color1) && COLOR.YELLOW.equals(color2)){
           System.out.println("Your colour combination creates orange!");
    }//....


}

Upvotes: 0

Matthew Clark
Matthew Clark

Reputation: 173

Another option that is available is for you to store them in a list. You can then check if a colour is in the list.

For example

String colourOne = "blue";
String colourTwo = "red";

List<String> colours= new ArrayList<String>(2);
colours.add(colourOne);
colours.add(colourTwo);


if((colours.contains("red") && colours.contains("blue")
    System.out.println("Your colour combination creates purple!");
etc

Upvotes: 0

Thilo
Thilo

Reputation: 262754

Maybe some booleans to remove the repetitions.

boolean isRed = colourOne.equalsIgnoreCase("red") || colourTwo.equalsIgnoreCase("red") ;
boolean isBlue =  colourOne.equalsIgnoreCase("blue") || colourTwo.equalsIgnoreCase("blue") ;

if (isRed && isBlue){
   // ...
} else if (isRed && isYellow) {
  // ...

Upvotes: 2

Related Questions