Choice Decision Maker (Random string from ArrayList)

Question

I'm learning about inheritance and polymorphism in my CS class and thought it'd be a good idea to find something I could apply myself to. I have a lot of ideas, and sometimes there's things I want to do and things I don't want to do. Why not leave the decision-making up to Java?

I'm looking for feedback to make my code better: whether it's something I could do with readability or inheritance, or even data structure wise. Or if there's bugs that'd be nice to know too :)

There are multiple classes extending MakeChoice, I just didn't put them here.

import java.util.*;

public class MakeChoice {

    // main
    public static void main(String[] args) {
        makeChoice makerShareChoice = new makeChoiceMakerShare();
        System.out.println(makerShareChoice);
    }

    public static int getChoiceForArrayList(int size) {
        int choice = generateRandomIntRange(0, size - 1);
        return choice;
    }

    public static int generateRandomIntRange(int min, int max) {
        int range = (max - min) + 1;
        int random = (int) (Math.random() * range + min);
        return random;
    }

}

import java.util.*;

public class MakeChoiceMakerShare extends makeChoice {
    private static ArrayList<String> choices = new ArrayList<String>();

    public makeChoiceMakerShare() {
        super();
        choices.add("acrylic LED");
        choices.add("MESA EduPi");
        choices.add("Collage of whiteboard wall");
        choices.add("sponge mitts");
        choices.add("hooks in closet");
        choices.add("10 x 10 LED Matrix");
        choices.add("What should I make? CoMotion addition to matrix");
        choices.add("What should I do this week? Screen");
        choices.add("Lasercraft belt + UW belt buckle + Wallet");
    }

    public String toString() {
        return "Your choice for this week in " + this.getClass() + " is: " + choices.get(getChoiceForArrayList(choices.size()));
    }
}

Answer 1

Thanks for sharing your code.

OOP

OOP doesn't mean to "split up" code into random classes.

The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.

Doing OOP means that you follow certain principles which are (amongst others):

• information hiding / encapsulation
• single responsibility
• separation of concerns
• KISS (Keep it simple (and) stupid.)
• DRY (Don't repeat yourself.)
• "Tell! Don't ask."
• Law of demeter ("Don't talk to strangers!")

class design

In OOP we create new classes if there is a change in behavior. You classes only change in properties. Therefore no extra class is justified.

overestimated inheritance

20 years ago we thought it would be a good idea to reuse code via inheritance so that a superclass provides common functionality for a bunch of subclasses.

Nowadays we Favor Composition over Inheritance. That means we inject common behavior as dependency to all classes needing it. This is because we have learned the hard way that inheritance of concrete classes does not support reuse of code in subclasses when you'd need to change the behavior in a base class.

Nevertheless we want an inheritance hierarchy for our classes so that we can benefit from polymorphism. We create that via interfaces which we have extending each other as needed.
_{(BTW: beware of Java8 default methods in interfaces! They are primarily meant to enable old interfaces with more than one method declaration to be used in lambda expressions...)}

Naming

Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

Please read (and follow) the Java Naming Conventions. The name of your base class starts with a lower case letter which should be rather upper case.

comments

Your code has one comment and this is useless.

Only add comments to tell your readers why the code is the way it is.

And before adding a comment think carefully if you could change some identifier names instead to make that code better understandable.

Magic numbers

your code has some magic numbers. This are literal values with a special meaning like here:

    int choice = generateRandomIntRange(0, size - 1);
    //...
    int range = (max - min) + 1;

You should introduce constants with meaningful names:

    public static final in LOWEST_INDEX = 0;
    public static final in ZERO_BASED_INDEX_CORRETION = 1;
    // ...
    int choice = generateRandomIntRange(LOWEST_INDEX, size - ZERO_BASED_INDEX_CORRETION);
    //...
    int range = (max - min) + ZERO_BASED_INDEX_CORRETION;

visibility scopes

carefully chose the visibility of your methods (and classes, properties should be private anyway...).

• Methods meant to be called by other code should be public
• Methods meant to be called (or overridden by) subclasses should be protected

Answer 2

-For starters, I'm not sure why you needed inheritance for this in the first place, as opposed to passing your choices array into your init function. I'm assuming just to get practice with inheritance?

-I'd move your toString function to your base class if this is functionality you plan on using in most of the children classes. I'd also rename this function to avoid common keywords and to be more descriptive--something like printResult might make more sense, unless your intent is to overload the built-in toString function.