0

My problem is that I have lots of If-Else statements. A LOT. It gets hard to read it, and I sometimes get output I don't want.

I have to compare 5 animals names, weights, and ages. As well as outputting and checking if they are same animal and display which are younger, older, weigh less, and weigh more. It works, sort of, but it is bad code design

Code:

 if(petOne.getName().equalsIgnoreCase(petTwo.getName()) && petThree.getName().equalsIgnoreCase(petFour.getName()) && petFour.getName().equalsIgnoreCase(petFive.getName()) )
    {
            System.out.println("All five dogs have the same name.");
    }
    else if(petOne.getWeight() == petTwo.getWeight() && petThree.getWeight() == petFour.getWeight() && petFour.getName() == petFive.getName())
    {
        System.out.println("All five dogs have the same weight");
    }
    else if(petOne.getAge() == petTwo.getAge() && petThree.getAge() == petFour.getAge() && petFour.getAge() == petFive.getAge())
    {
        System.out.println("All five dogs are the same age.");
    }
    //Below check if any pets are the same
    //1
     if(petOne.toString().equals(petTwo.toString()))
    {
        System.out.println(petOne.toString()+" and "+petTwo.toString()+" are the same");
    }
    else if(petOne.toString().equals(petThree.toString()))
    {
        System.out.println(petOne.toString()+" and "+petThree.toString()+" are the same");
    }
    else if(petOne.toString().equals(petFour.toString()))
    {
        System.out.println(petOne.toString()+" and "+petFour.toString()+ "are the same");
    }
    else if(petOne.toString().equals(petFive.toString()))
    {
        System.out.println(petOne.toString()+" and "+petFive.toString()+" are the same");
    }
    //2
        if(petTwo.toString().equals(petOne.toString()))
    {
        System.out.println(petTwo.toString()+" and "+petOne.toString()+" are the sae");
    }
    else if(petTwo.toString().equals(petThree.toString()))
    {
        System.out.println(petTwo.toString()+" and "+petThree.toString()+" are the same");
    }
    else if(petTwo.toString().equals(petFour.toString()))
    {
        System.out.println(petTwo.toString()+" and "+petFour.toString()+" are the same");
    }
    else if(petTwo.toString().equals(petFive.toString()))
    {
        System.out.println(petTwo.toString()+" and "+petFive.toString()+" are the same");
    }
    //3
        if(petThree.toString().equals(petOne.toString()))
    {
        System.out.println(petThree.toString()+" and "+petOne.toString()+" are the same");
    }
     else if(petThree.toString().equals(petTwo.toString()))
    {
        System.out.println(petThree.toString()+" and "+petTwo.toString()+" are the same");
    }
    else if(petThree.toString().equals(petFour.toString()))
    {
        System.out.println(petThree.toString()+" and "+petFour.toString()+" are the same");
    }
    else if(petThree.toString().equals(petFive.toString()))
    {
        System.out.println(petThree.toString()+" and "+petFive.toString()+" are the same");
    }
    //4
        if(petFour.toString().equals(petOne.toString()))
    {
        System.out.println(petFour.toString()+" and "+petOne.toString()+" are the same");
    }
    else if(petFour.toString().equals(petTwo.toString()))
    {
        System.out.println(petFour.toString()+" and "+petTwo.toString()+" are the same");
    }
    else if(petFour.toString().equals(petThree.toString()))
    {
        System.out.println(petFour.toString()+" and "+petThree.toString()+" are the same");
    }
    else if(petFour.toString().equals(petFive.toString()))
    {
        System.out.println(petFour.toString()+" and "+petFour.toString()+" are the same");
    }
    //5
        if(petFive.toString().equals(petOne.toString()))
    {
        System.out.println(petFive.toString()+" and "+petOne.toString()+" are the same");
    }
    else if(petFive.toString().equals(petTwo.toString()))
    {
        System.out.println(petFive.toString()+" and "+petTwo.toString()+" are the same");
    }
    else if(petFive.toString().equals(petThree.toString()))
    {
        System.out.println(petFive.toString()+" and "+petThree.toString()+" are the same");
    }
    else if(petFive.toString().equals(petFour.toString()))
    {
        System.out.println(petFive.toString()+" and "+petFour.toString()+" are the same");
    }
    //Below caculates which pet is the youngest

        if(petOne.getAge() < petTwo.getAge() && petOne.getAge() < petThree.getAge() && petOne.getAge() < petFour.getAge() && petOne.getAge() < petFive.getAge())
    {
        System.out.println(petOne.getName()+" is the youngest pet.");
    }
    else if(petTwo.getAge() < petOne.getAge() && petTwo.getAge() < petThree.getAge() && petTwo.getAge() < petFour.getAge() && petTwo.getAge() < petFive.getAge())
    {
        System.out.println(petTwo.getName()+" is the youngest pet.");
    }
    else if(petThree.getAge() < petOne.getAge() && petThree.getAge() < petTwo.getAge() && petThree.getAge() < petFour.getAge() && petThree.getAge() < petFive.getAge())
    {
        System.out.println(petThree.getName()+" is the youngest pet.");
    }
    else if(petFour.getAge() < petOne.getAge() && petFour.getAge() < petTwo.getAge() && petFour.getAge() < petThree.getAge() && petFour.getAge() < petFive.getAge())
    {
        System.out.println(petFour.getName()+" is the youngest pet.");
    }
    else if(petFive.getAge() < petOne.getAge() && petFive.getAge() < petTwo.getAge() && petFive.getAge() < petThree.getAge() && petFive.getAge() < petFour.getAge())
    {
        System.out.println(petFive.getName()+" is the youngest pet.");
    }
    //Below caculates which pet is the oldest 
        if(petOne.getAge() > petTwo.getAge() && petOne.getAge() > petThree.getAge() && petOne.getAge() > petFour.getAge() && petOne.getAge() > petFive.getAge())
    {
        System.out.println(petOne.getName()+" is the oldest pet.");
    }
     else if(petTwo.getAge() > petOne.getAge() && petTwo.getAge() > petThree.getAge() && petTwo.getAge() > petFour.getAge() && petTwo.getAge() > petFive.getAge())
    {
        System.out.println(petTwo.getName()+" is the oldest pet.");
    }
     else if(petThree.getAge() > petOne.getAge() && petThree.getAge() > petTwo.getAge() && petThree.getAge() > petFour.getAge() && petThree.getAge() > petFive.getAge())
    {
        System.out.println(petThree.getName()+" is the oldest pet.");
    }
     else if(petFour.getAge() > petOne.getAge() && petFour.getAge() > petTwo.getAge() && petFour.getAge() > petThree.getAge() && petFour.getAge() > petFive.getAge())
    {
        System.out.println(petFour.getName()+" is the oldest pet.");
    }
     else if(petFive.getAge() > petOne.getAge() && petFive.getAge() > petTwo.getAge() && petFive.getAge() > petThree.getAge() && petFive.getAge() > petFour.getAge())
    {
        System.out.println(petFive.getName()+" is the oldest pet.");
    }
    //Below caculates which pet is the smallest
        if(petOne.getWeight() < petTwo.getWeight() && petOne.getWeight() < petThree.getWeight() && petOne.getWeight() < petFour.getWeight() && petOne.getWeight() < petFive.getWeight())
    {
        System.out.println(petOne.getName()+" is the smallest pet.");
    }
     else if(petTwo.getWeight() < petOne.getWeight() && petTwo.getWeight() < petThree.getWeight() && petTwo.getWeight() < petFour.getWeight() && petTwo.getWeight() < petFive.getWeight())
    {
        System.out.println(petTwo.getName()+" is the smallest pet.");
    }
     else if(petThree.getWeight() < petOne.getWeight() && petThree.getWeight() < petTwo.getWeight() && petThree.getWeight() < petFour.getWeight() && petThree.getAge() < petFive.getWeight())
    {
        System.out.println(petThree.getName()+" is the smallest pet.");
    }
     else if(petFour.getWeight() < petOne.getWeight() && petFour.getWeight() < petTwo.getWeight() && petFour.getWeight() < petThree.getWeight() && petFour.getWeight() < petFive.getWeight())
    {
        System.out.println(petFour.getName()+" is the smallest pet.");
    }
     else  if(petFive.getWeight() < petOne.getWeight() && petFive.getWeight() < petTwo.getWeight() && petFive.getWeight() < petThree.getWeight() && petFive.getWeight() < petFour.getWeight())
    {
        System.out.println(petFive.getName()+" is the smallest pet.");
    }
    //Below caculates to see which pet is the largest
        if(petOne.getWeight() > petTwo.getWeight() && petOne.getWeight() > petThree.getWeight() && petOne.getWeight() > petFour.getWeight() && petOne.getWeight() > petFive.getWeight())
    {
        System.out.println(petOne.getName()+" is the largest pet.");
    }
     else if(petTwo.getWeight() > petOne.getWeight() && petTwo.getWeight() > petThree.getWeight() && petTwo.getWeight() > petFour.getWeight() && petTwo.getWeight() > petFive.getWeight())
    {
        System.out.println(petTwo.getName()+" is the largest pet.");
    }
     else if(petThree.getWeight() > petOne.getWeight() && petThree.getWeight() > petTwo.getWeight() && petThree.getWeight() > petFour.getWeight() && petThree.getAge() > petFive.getWeight())
    {
        System.out.println(petThree.getName()+" is the largest pet.");
    }
    else if(petFour.getWeight() > petOne.getWeight() && petFour.getWeight() > petTwo.getWeight() && petFour.getWeight() > petThree.getWeight() && petFour.getWeight() > petFive.getWeight())
    {
        System.out.println(petFour.getName()+" is the largest pet.");
    }
    else if(petFive.getWeight() > petOne.getWeight() && petFive.getWeight() > petTwo.getWeight() && petFive.getWeight() > petThree.getWeight() && petFive.getWeight() > petFour.getWeight())
    {
        System.out.println(petFive.getName()+" is the largest pet.");
    }
Makoto
  • 104,088
  • 27
  • 192
  • 230
EOxJ
  • 102
  • 7
  • 6
    petOne, petTwo, petThree -- your code is crying for an array or an ArrayList, and then loop with a for loop or nested for-loops if need be. This will help shorting things considerably. – Hovercraft Full Of Eels Dec 05 '17 at 22:22
  • Are you allowed to use Maps? Also, you can make your code a little easier to read by removing all toString() calls in your print statements; for example, `System.out.println(petOne + " and " + petTwo + " are the same");`. (toString is called implicitly when concatenating strings in Java.) – VGR Dec 05 '17 at 22:30
  • 2
    I'd suggest two basic changes. Firstly, as @Hovercraft pointed out, you should keep the Pets in some kind of structure that you can iterate through. I'd recommend an ArrayList, because it's a fairly easy kind of structure to work with. Secondly, write small methods that do one task only, for example, one method to find the oldest pet, another to find the heaviest pet, another to check for duplicate names and so on. Each of these methods will iterate through the collection, and do something with the pets that it finds there. – Dawood ibn Kareem Dec 05 '17 at 22:40
  • 1
    Addition to all of the above. Don't check if petN is the oldest by checking if it is older than all other pets. That creates N*N comparisons. Instead, find `oldestAge`, `youngestAge`, `heaviestWeight`, `lightestWeight`, etc. That should only be N comparisons per category. Then you can simply check if petN's weight and check if it is equal to the `lightestWeight`, etc. Again, only N comparisons per category. – AJNeufeld Dec 05 '17 at 23:06
  • And think about what should happen if two pets are equal oldest, or equal heaviest, etc. – Dawood ibn Kareem Dec 05 '17 at 23:16
  • Like all of you have pointed out and said. I have made changes, but now, I am having some more trouble with comparing them and outputting the correct information – EOxJ Dec 05 '17 at 23:22
  • Each block that starts `if (test == false)` needs to be _after_ the corresponding `for` loop, not inside it. Otherwise, that block will run multiple times. – Dawood ibn Kareem Dec 05 '17 at 23:30
  • So I realized I had answered the wrong question, essentially. We don't encourage changing your existing post so dramatically. If you have a new question, you may ask a new question instead. – Makoto Dec 05 '17 at 23:38

1 Answers1

0

Assuming the following data structure used to store pets:

List<Pet> pets = new ArrayList<>();

and assuming sensible definitions for toString and equals, your queries can be handled through the use of streams and filters.

// Case 1:  Find all equivalent pets
List<Pet> equivalentPets = pets.stream()
                   .distinct()
                   .collect(Collectors.toList());

// Case 2:  Find the heaviest pet
Pet heaviestPet = pets.stream()
                      .max(Comparator.comparing(Pet::getWeight))
                      .get();

The rest is left as an exercise for the reader. The main objective is to iterate over the collection instead of repeating yourself. You're more prone to errors the more you repeat yourself.

Makoto
  • 104,088
  • 27
  • 192
  • 230