A menu option that receives input from users and then performs actions

Please review my FindMovie and AddMovie methods.

public static boolean FindMovie(String movietofind, String[] ArrayMovie) {
    movie Movieobject = new movie();

    for (String ArrayMovie1 : ArrayMovie) {
        if (ArrayMovie1.equals(movietofind)) {
            return true;
        }
    }
    return false;
}

public static void AddMovie(int i, String[] ArrayMovie, String[] UserRating, int[] UserReview) {
    int addmovie, userrating;
    boolean Movie1;
    String MPAA;
    Scanner console = new Scanner(System.in);

    System.out.println("Please give me the name of the movie you want to add.");
    String Movie = console.next();

    Movie1 = FindMovie(Movie, ArrayMovie);
    if (Movie1 = false) {
        System.out.println("Do you want to add this movie to the collection? Please select 1 for Yes or 2 for No");
        addmovie = console.nextInt();
        if (addmovie == 1) {
            ArrayMovie[i] = Movie;

            System.out.println("What is the type of MPAA rating of the movie?");
            System.out.println("Please enter G, PG, PG13, or R");
            MPAA = console.next();
            UserRating[i] = MPAA;

            System.out.println("What is your rating for movie?");
            System.out.print("please choose 1, 2, 3, 4, or 5");
            userrating = console.nextInt();
            UserReview[i] = userrating;

        } else if (addmovie == 2) {
        }
    }
}

Technical

• Your naming conventions are inconsistent - you mix PascalCase with alllowercase (don't know if that convention has it's own name). In Java types (like classes and interfaces) are PascalCase and method and variable names are camelCase.

• Declare variables where they are used and not all at the top of the function.

• Movie1 one is not a good name for a variable. The variable name should express the meaning of it. In this case it should be movieExists.

• FindMovie won't compile due to the first line in the function (there is no movie type in your source code and nothing actually references a movie type except that one line of code). It should be removed (Movieobject is never used)

• There is no need to prefix the variable name with the type of the variable it just adds clutter and no real value. So ArrayMovie should be movies as that's what the collection represents (a collection of movies).

• If you use ArrayList instead of arrays you get automatic resizing when adding new movies and a whole bunch of nice utility functions like contains which reduce a lot of code clutter and does not impose the limitation of fixed sized arrays.

• AddMovie requires the caller to pass in the index in the array where the movie should be stored. This is bad as it requires the caller to keep information about the structure of the list. It should just add a new movie to the end of a list (see previous point about ArrayList).

-
You can reduce nesting by breaking early when hitting the negative condition, for example:

bool movieExists = FindMovie(Movie, ArrayMovie);
if (movieExists)
{
    return;
}

System.out.println("Do you want to add this movie to the collection? Please select 1 for Yes or 2 for No");
bool userAction = console.nextInt();
if (userAction != 1)
{
    return;
}

// add the movie

Design

It seems like you had the right idea at some point about having a movie class (although it should be named Movie) encapsulating the properties of a movie like name, rating and reviews. Right now you seem to pass them around as 3 independent lists and the only connection is that the index in each list refers to the same movie. This is easy to get out of sync and you always have to pass around 3 lists - not to mention what happens when you want to add a 4th or 5th property.

AddMovie should just accept an ArrayList as input to which you can add a new movie after you have gathered all the user input.

StackExchange Code Review Q#35665, answer score: 4