Having trouble debugging variable might not have been initialized error
I have seen a lot of similar questions in the archives, but I can't find a scenario like the problem I'm having.
Below is my code. I'm running into errors with "finalPrice" and "grandTotalPrice" may not have been initialized. The lines of code are towards the end of the program.
The variables should be assigned totals via console input from above. I'm not sure what the error is, or why. Can anyone please help me out, and explain?
Code:
import java.util.*;
public class PictureFrames
{
static Scanner console = new Scanner(System.in);
static final double REGULAR_FRAME = .15, FANCY_FRAME = .25;
static final double COLOR = .10, CARDBOARD = .02, GLASS = .07, CROWNS = .35;
public static void main (String[] args)
{
double length, width, area, perimeter;
double priceOfFrame, priceOfColor, priceOfCardboard, priceOfGlass, priceOfCrowns, finalPrice, crownFinalPrice, grandTotalPrice;
int numberOfCrowns;
char typeOfFrame, choiceOfColor, choiceOfCrowns;
System.out.println ("Please enter the length of your picure in inches:");
length = console.nextDouble();
System.out.println ("Please enter the width of your picure in inches: ");
width = console.nextDouble();
System.out.println ("Please enter the type of frame: R or r (Regular), F or f (Fancy). ");
typeOfFrame = console.next().开发者_如何学CcharAt(0);
System.out.println ("Would you like to add color?: Y for (Yes), N for (No): ");
choiceOfColor = console.next().charAt(0);
switch (typeOfFrame)
{
case 'R':
case 'r':
if (choiceOfColor == 'N')
{
area = (length * width);
perimeter = (2 * length) + (2 * width);
priceOfFrame = (perimeter * REGULAR_FRAME);
priceOfCardboard = (area * CARDBOARD);
priceOfGlass = (area * GLASS);
finalPrice = (priceOfFrame + priceOfCardboard + priceOfGlass);
break;
}
else if (choiceOfColor == 'Y')
{
area = (length * width);
perimeter = (2 * length) + (2 * width);
priceOfColor = (area * COLOR);
priceOfFrame = (perimeter * REGULAR_FRAME);
priceOfCardboard = (area * CARDBOARD);
priceOfGlass = (area * GLASS);
finalPrice = (priceOfFrame + priceOfColor + priceOfCardboard + priceOfGlass);
break;
}
case 'F':
case 'f':
if (choiceOfColor == 'N')
{
area = (length * width);
perimeter = (2 * length) + (2 * width);
priceOfFrame = (perimeter * FANCY_FRAME);
priceOfCardboard = (area * CARDBOARD);
priceOfGlass = (area * GLASS);
finalPrice = (priceOfFrame + priceOfCardboard + priceOfGlass);
break;
}
else if (choiceOfColor == 'Y')
{
area = (length * width);
perimeter = (2 * length) + (2 * width);
priceOfColor = (area * COLOR);
priceOfFrame = (perimeter * FANCY_FRAME);
priceOfCardboard = (area * CARDBOARD);
priceOfGlass = (area * GLASS);
finalPrice = (priceOfFrame + priceOfColor + priceOfCardboard + priceOfGlass);
break;
}
}
System.out.println ("Would you like to add crowns? Enter Y (Yes), or N (No): ");
choiceOfCrowns = console.next().charAt(0);
if (choiceOfCrowns == 'Y')
{
System.out.println ("How many crowns would you like? ");
numberOfCrowns = console.nextInt();
crownFinalPrice =(numberOfCrowns * CROWNS);
grandTotalPrice = (crownFinalPrice + finalPrice);
}
else if (choiceOfCrowns == 'N')
System.out.printf ("Your total comes to: $%.2f%n", grandTotalPrice);
}
}
Here is a first-pass refactoring of your code, addressing your specific question as well as a few others. Some observations that were addressed in the refactoring:
In response to your question (and some of the other answers) don't get in the habit of blindly initializing variables just to make the compiler errors go away. The compiler error here is an indication that you need to clearly define the variable (meaningfully for your application) regardless of the potential states. Initializing the variable just hides the problem that you have uncontrolled/unexpected states that haven't been dealt with.
The
main()method is too long and has too many things in it. The refactoring I show is a good example of a "next step," by no means a completed process... but the first thing was to pull out some of the logic into a digestible subset. Software construction relies on breaking things into components you can reason about, and why not start with your frame price computer?Related to the previous point, you need to work to scope variables down to the absolute narrowest possible scope. Locality is your friend. Minimize variable scope. This is aided by breaking things into separate methods, but it also applies to bringing variables into blocks, etc. Here's a good related SO discussion. Also, Effective Java Item 45: Minimize the scope of local variables.
This one is controversial, but I'll offer my opinion. Use
finaleverywhere you can. Mutability is not your friend, in most cases, and this code is a great example -- you've got bits and pieces taking on values in a lot of different places, when in fact most of the values can be defined once and categorically, and the other computations can proceed linearly. Its more readable/maintainable, and you'll be amazed at the number of logical errors you'll find at the compile stage rather than in iterative debugging by doing this. Favor immutability, allow mutability judiciously and only with cause. See my answer on this question for some discussion, and several related, linked SO questions. Also, Effective Java Item 15: Minimize Mutability.Prefer
enumtoStringorcharor whatever. Especially when coping with potentially misbehaving input from a user or external system, your overriding goal should be to reduce the state space to the absolute minimum, and get things into a controlled vocabulary. I did a first pass here, but your code should throw exceptions or emit error messages to the user (depending on the level of abstraction; here emitting errors and asking again for input) as close to the interface as is possible, not allowing unstructured/dirty data to propagate deeper into the application than is necessary. The deeper it goes, the less context you have, and the uglier/messier the logic to cope with a failure becomes. Then, you accrete error handling logic that makes dead-simple internal computations untestable.Duplication is a code smell. If you're computing the area in two different places, refactor it to avoid. Related on SO: "What duplication threshold...?", but the duplication threshold for something as simple as area = length x width is EXACTLY once. Minimize or eliminate duplicate code.
Use
Exceptionfor exceptional conditions. This actually closes the loop with my first point and your question. Your code had variables which the compiler wasn't sure were being initialized because there was no well-defined value for them to be initialized to in the exceptional cases -- they represented erroneous input, not a compiler warning to squash. Get the state space under control, and when an unintended state is encountered, THROW AN EXCEPTION. Better an explicit and interpretable, potentially recoverable error than a silent but incorrect result.
There's more, but that's a useful set of critiques, I think, and more than you asked for. The code:
import java.util.*;
public final class PictureFrames
{
static Scanner console = new Scanner(System.in);
static final double REGULAR_FRAME = .15, FANCY_FRAME = .25;
static final double COLOR = .10, CARDBOARD = .02, GLASS = .07, CROWNS = .35;
enum FrameType {
/** Regular. */
R,
/** Fancy. */
F;
};
static double areaPriceInDollars(final FrameType frameType,
final double length,
final double width,
final boolean color)
{
final double area,perimeter,
priceOfFrame,
priceOfCardboard,
priceOfGlass,
priceOfColor;
area = length * width;
perimeter = 2 * (length + width);
priceOfCardboard = (area * CARDBOARD);
priceOfGlass = (area * GLASS);
if (color)
priceOfColor = area * COLOR;
else
priceOfColor = 0.0;
switch (frameType) {
case R:
priceOfFrame = (perimeter * REGULAR_FRAME);
break;
case F:
priceOfFrame = (perimeter * FANCY_FRAME);
break;
default:
throw new IllegalArgumentException("FrameType "+frameType+" unknown, no price available.");
}
return priceOfColor + priceOfCardboard + priceOfGlass + priceOfFrame;
}
public static void main(String[] args)
{
System.out.println("Please enter the length of your picure in inches:");
final double length = console.nextDouble();
System.out.println("Please enter the width of your picure in inches: ");
final double width = console.nextDouble();
System.out
.println("Please enter the type of frame: R or r (Regular), F or f (Fancy). ");
final char typeOfFrame = console.next().charAt(0);
FrameType frameType = FrameType.valueOf(""
+ Character.toUpperCase(typeOfFrame));
System.out
.println("Would you like to add color?: Y for (Yes), N for (No): ");
final char choiceOfColor = console.next().charAt(0);
final boolean color = Character.toUpperCase(choiceOfColor) == 'Y';
System.out
.println("Would you like to add crowns? Enter Y (Yes), or N (No): ");
final char choiceOfCrowns = console.next().charAt(0);
final boolean crowns = Character.toUpperCase(choiceOfCrowns) == 'Y';
final double priceOfCrowns;
if (crowns) {
System.out.println("How many crowns would you like? ");
final int numberOfCrowns = console.nextInt();
priceOfCrowns = (numberOfCrowns * CROWNS);
} else {
priceOfCrowns = 0.0;
}
final double grandTotalPrice = priceOfCrowns
+ areaPriceInDollars(frameType, length, width, color);
System.out.printf("Your total comes to: $%.2f%n", grandTotalPrice);
}
}
What happens if typeOfFrame is not R, r, F, or f? A possible solution is to add a "default" case to the switch.
I think this code is closer to what you want (I reduced a lot of duplication). Note that I did not really deal with errors (the calls to isValidChoice and isFrameType methods are a start with how to deal with them). The issue above was that you are basically assuming that the user will only ever enter in valid input and forgetting that there are times that, even though, Y and N are the only valid choices they could enter in Q. I also fixed a bug where you were not always printing out the grand total, and were not setting the grand total (the last part above the printf).
The last suggestion I have is do NOT use doubles for this (unless you were told to by your instructor) because they will give you inaccurate numbers in some cases. Take a look at java.math.BigDecimal instead.
class Main
{
static Scanner console = new Scanner(System.in);
static final double REGULAR_FRAME = .15;
static final double FANCY_FRAME = .25;
static final double COLOR = .10;
static final double CARDBOARD = .02;
static final double GLASS = .07;
static final double CROWNS = .35;
public static void main (String[] args)
{
final double length;
final double width;
final char typeOfFrame;
final char choiceOfColor;
System.out.println ("Please enter the length of your picure in inches:");
length = console.nextDouble();
System.out.println ("Please enter the width of your picure in inches: ");
width = console.nextDouble();
System.out.println ("Please enter the type of frame: R or r (Regular), F or f (Fancy). ");
typeOfFrame = console.next().charAt(0);
System.out.println ("Would you like to add color?: Y for (Yes), N for (No): ");
choiceOfColor = console.next().charAt(0);
if(!(isFrameType(typeOfFrame)))
{
}
else
{
final double area;
final double perimeter;
final double priceOfFrame;
final double priceOfCardboard;
final double priceOfGlass;
area = (length * width);
perimeter = (2 * length) + (2 * width);
priceOfFrame = (perimeter * REGULAR_FRAME);
priceOfCardboard = (area * CARDBOARD);
priceOfGlass = (area * GLASS);
if(isValidChoice(choiceOfColor))
{
final double priceOfColor;
final double finalPrice;
final char choiceOfCrowns;
final double grandTotalPrice;
if(choiceOfColor == 'N')
{
finalPrice = (priceOfFrame + priceOfCardboard + priceOfGlass);
}
else
{
priceOfColor = (area * COLOR);
finalPrice = (priceOfFrame + priceOfColor + priceOfCardboard + priceOfGlass);
}
System.out.println ("Would you like to add crowns? Enter Y (Yes), or N (No): ");
choiceOfCrowns = console.next().charAt(0);
if(isValidChoice(choiceOfCrowns))
{
if(choiceOfCrowns == 'Y')
{
final double crownFinalPrice;
final int numberOfCrowns;
System.out.println ("How many crowns would you like? ");
numberOfCrowns = console.nextInt();
crownFinalPrice =(numberOfCrowns * CROWNS);
grandTotalPrice = (crownFinalPrice + finalPrice);
}
else
{
grandTotalPrice = finalPrice;
}
System.out.printf ("Your total comes to: $%.2f%n", grandTotalPrice);
}
}
}
}
private static boolean isFrameType(final char c)
{
final char lower;
lower = Character.toLowerCase(c);
return (lower == 'r' || lower == 'f');
}
private static boolean isValidChoice(final char c)
{
return (c == 'Y' || c == 'N');
}
}
This is happening because the compiler is warning you that the values might never have been set during some runs of the application. They are set in the switch and if statements, but the compiler warns you that some input may lead to them not being initialized.
Simply setting them to a value when you define them would be enough:
double finalPrice = 0, grandTotalPrice = 0;
but you could also ensure that they are set no matter what path the application takes (knowning all possible paths is good practice anyway).
加载中,请稍侯......
精彩评论