macOS [help!] java program - one more final touch

makbarg · Oct 4, 2010

i cant find out how to solve this program error:
it says break cannot be used outside of loop or switch.. but still...

Code:

import java.util.*;
public class Store {
	
	public static void main (String[] args){
	Scanner console = new Scanner(System.in);
	
	String 	movieTitle, status, userIn;
	double 	movieLength;
	int i; 
	int indexRent = 0;
	int indexReturn = 0;
	
	
	DVD dvd[]	= new DVD [6];
	dvd[0] 		= new DVD ("Iron Man",3.5,"In");
	dvd[1] 		= new DVD ("Death Note",2.5,"In");
	dvd[2] 		= new DVD ("Shrek",2,"In");
	dvd[3] 		= new DVD ("Transformer",3.25,"In");
	dvd[4] 		= new DVD ("The Eye",3,"In");
	dvd[5]	 	= new DVD ("Fantastic 4",4,"In");
	
	System.out.println("ENTER COMMAND:");
	System.out.println("A) Renting DVD(s)");
	System.out.println("B) Returning DVD(s)");
	System.out.println("C) All DVD informations");
	System.out.println("D) Exit program");
	System.out.println("Please enter your choice: ");
	
	userIn = console.nextLine();
	
		if (userIn.compareTo("A") == 0 || userIn.compareTo("a") == 0)	{
			System.out.println("Enter DVD title: ");
			String movieTitleReturn = console.nextLine();
				for(i=0; i<dvd.length; i++)	
					{
					if(movieTitleReturn.equals(dvd[i].getMovieTitle()));
					indexRent = i;
					break;
					}
				if(indexRent == i)
					{
					dvd[indexRent].dvdRent();
					}
				
				else
					{
					System.out.println("Invalid input - back to main menu");
					}
				
		}
		
		else if (userIn.compareTo("b") == 0 || userIn.compareTo ("B") == 0)	{
			
			System.out.println("Enter DVD title: ");
			String movieTitleReturn = console.nextLine();
			for(i=0; i<dvd.length; i++)
				{
				if(movieTitleReturn.equals(dvd[i].getMovieTitle()));
				indexReturn = i;
				break;
				}
			if(indexReturn == i)
				{
				dvd[indexReturn].dvdReturn();
				}
			
			else
				{
				System.out.println("Process cannot be done - wrong movie title");
				}

		}
		
		else if (userIn.compareTo("c") == 0 || userIn.compareTo ("C") == 0)	{
			for(i=0; i == dvd.length; i++)
				{
				dvd[i].printDetails();
				}
			
		}
		
		else if (userIn.compareTo("d") == 0 || userIn.compareTo ("D") == 0)	{
			
			System.out.println("Exiting program");
			break;
			}
		else
			{
			System.out.println("Input is invalid");
			}
	}
}

thanks for your time.

naples98 · Oct 4, 2010

Take another look at each of the for loops where you use break. Neither one of those loops ever fully executes which is probably why you are getting the error.

Follow the logic in each for loop and try to figure out why they don't ever execute at least one time through.

elppa · Oct 4, 2010

Use System.exit(0) instead of break on line 77.

You are not in a loop at that stage.

naples98 · Oct 4, 2010

elppa said:
Use System.exit(0) instead of break on line 77.

You are not in a loop at that stage.

I missed the break on line 77 but you don't need to use System.exit(0) or break on that line.

plinden · Oct 4, 2010

It's unrelated to the problem you mention here, but this loop doesn't look right. See if you can work out what i is at the break.

Code:

	for(i=0; i<dvd.length; i++)	
	{
		if(movieTitleReturn.equals(dvd[i].getMovieTitle()));
		indexRent = i;
		break;
	}

lee1210 · Oct 4, 2010

There's plenty going on here... naples98 has pointed out a few serious problems already. I will just state that you should always use {} for the body of an if. Using a few extra lines of code is OK, and it will help avoid the problem you just encountered.

Also:

Code:

for(i=0; i == dvd.length; i++)

This is very unlikely what you want. Execution will stop whenever i is not equal to the length of your array. (looks like plinden already mentioned this)

movieTitle, status, and movieLength are never used. May as well get rid of them to avoid confusion.

indexRent and indexReturn are only initialized once. Initialize them each time you are going to use them.

elppa told you why you got the error relating to the use of break outside of a loop... but i think it's not wrong that you have a break there, but rather that you're missing a loop. Right now, when the user runs this they get to perform exactly one action. This means messages like "back to the main menu" are a lie. You need a big loop for the main menu, at which point that break will do exactly what you want.

-Lee

macsmurf · Oct 4, 2010

You don't need to declare variables in the beginning of the method as in C. Declare them when you need them. There are several variables you don't use.

You don't need to iterate through an array using an integer as in C. Use foreach instead (see code). Better yet, use a List instead of an array.

Nested loops and if statements + breaks are evil. They make the code hard to read and hard to debug. Use private methods instead.

Normalize the input before you test it (toLowercase in this instance);

Don't use compareTo when you don't need to establish ordering. Use equals() instead.

Don't prefix all your methods in DVD with dvd. It is already apparent that we're talking about DVDs. return() is an invalid method name in Java so I would use checkIn()/checkOut() instead.

Don't Repeat Yourself (This is called the DRY principle). Extract duplicated code into methods.

Use a while loop to check for exit command.

Use proper indentation. In other words, follow the Java coding guidelines.

Use methods such as isIn() to check if a movie is in. Don't use text strings in the constructor to specify whether a movie is in or out. Use a boolean (or just assume that it's in at construction time).

I've made a new version of your program. I would probably make a Store class and make a tiny Main class to start it up, but it should show you what I mean. Ask yourself: which version would you rather read/debug? (I haven't handled the case that a movie can be rented out several times.)

EDIT: I've removed most of the code to avoid you risking being accused of cheating. The comments and the below should get you started

Code:

import java.util.Scanner;

public class Store {

    private DVD[] dvds = {
            new DVD("Iron Man", 3.5, true),
            new DVD("Death Note", 2.5, true),
            new DVD("Shrek", 2, true),
            new DVD("Transformer", 3.25, true),
            new DVD("The Eye", 3, true),
            new DVD("Fantastic 4", 4, true)
    };

    public static void main(String[] args) {
        Store store = new Store();
        store.execute();
    }

    private void execute() {
        Scanner console = new Scanner(System.in);
        String mainCommand = getMainCommandFromUser(console);

        while (!mainCommand.equals("d")) {
            if (mainCommand.equals("a")) {
                rentCommand(console);
            } else if (mainCommand.equals("b")) {
                returnCommand(console);
            } else if (mainCommand.equals("c")) {
                printDetails();
            } else {
                invalidInput();
            }
            mainCommand = getMainCommandFromUser(console);
        }

        System.out.println("Exiting program");
    }

    private String getMainCommandFromUser(Scanner console) {
        printMainMenu();
        return console.nextLine();
    }

    private void printMainMenu() {
        System.out.println("");
        System.out.println("ENTER COMMAND:");
        System.out.println("A) Renting DVD(s)");
        System.out.println("B) Returning DVD(s)");
        System.out.println("C) All DVD informations");
        System.out.println("D) Exit program");
        System.out.println("Please enter your choice: ");
    }

    // TODO

    private void invalidInput() {
        System.out.println("Invalid input");
    }
}

chown33 · Oct 4, 2010

macsmurf said:
... I've made a new version of your program. ...

This is obviously a homework program. By writing it for them, you've effectively made the OP into a plagiarist or cheater if they hand in this program as their own.

The usual practice here is to explain what to fix or change, but to leave the actual programming to the OP.

And I agree with most of your suggestions, except they may not have been taught or shown the foreach loop yet. Ditto for the use of List. And I wouldn't use an if-else chain on a String, but a switch on first letter.

macsmurf · Oct 4, 2010

chown33 said:
This is obviously a homework program. By writing it for them, you've effectively made the OP into a plagiarist or cheater if they hand in this program as their own.

The usual practice here is to explain what to fix or change, but to leave the actual programming to the OP.

I agree somewhat and I would normally not do that. However, the code is so badly broken that I felt the only way to explain the issues was to make an example. All my points are general in nature. Also, the guy has obviously worked at it for a while. In addition, the code is not complete and still have some issues that I didn't mention.

Furthermore, it is my experience that beginning programming classes don't focus on good code but more on working code.

But you have a point, of course

And I agree with most of your suggestions, except they may not have been taught or shown the foreach loop yet. Ditto for the use of List. And I wouldn't use an if-else chain on a String, but a switch on first letter.

The main problem is that you can't make switches on strings in Java.

mrbash · Oct 4, 2010

macsmurf said:
The main problem is that you can't make switches on strings in Java.

I think a beta for Java7 is slated for release mid October, and will get that feature.

ulbador · Oct 4, 2010

macsmurf said:
The main problem is that you can't make switches on strings in Java.

If I was going to do this I would probably use static ints, or maybe even the hashcode of the string...

Code:

String myColor = "red";

switch (myColor.hashcode()) {

case "red".hashcode():
System.out.println("You had red");
break;


}

If you are doing just single characters instead of multiple characters (two different data types), you can probably switch directly on it.

case 'a' not case "a".. Single quotes will tell it that it is a character and not a string.

If that doesn't work, a single character can be converted directly to an integer:

The getNumericValue() method of the Character object should be what you need.

macsmurf · Oct 4, 2010

mrbash said:
I think a beta for Java7 is slated for release mid October, and will get that feature.

Yeah, I've read that too. However, closures have been booted out again, which is extremely disappointing. Don't get me started

chown33 · Oct 4, 2010

macsmurf said:
The main problem is that you can't make switches on strings in Java.

Which is why I said "on first letter", which would be a char, which can be used as a case value. And for better similarity between the main-menu's instructions, which use upper-case letters, I would have used upper-case as the canonical form. Example:

Code:

case 'A': ..code here..
   break;
case 'B': ..code here..
   break;

This assumes, however, that switch/case has been taught.

Search

Search

macOS [help!] java program - one more final touch

makbarg

macrumors newbie

naples98

macrumors member

elppa

macrumors 68040

naples98

macrumors member

plinden

macrumors 601

lee1210

macrumors 68040

macsmurf

macrumors 65816

chown33

Moderator

macsmurf

macrumors 65816

mrbash

macrumors 6502

ulbador

macrumors 68000

macsmurf

macrumors 65816

chown33

Moderator

Our Staff