Mike Gershman
SCJP 1.4, SCWCD in process
At a glance, your program looks technically correct, but your declaration of round() is misleading.
Also, when you get a bad value, it's easier to test your program if you set the default value rather than calling System.exit().
Were you told to reject negative temperatures? That prevents the most inteesting test cases, like -459.67F = -273.15C = absolute zero.
Mike Gershman
SCJP 1.4, SCWCD in process
Your declaration of round() has a formal argument and a return value, but you actually ignore the formal argument roundTempValue and instead use the instance variable tempValue. You return a result, but you also create a "side effect" by directly changing the instance variable tempValue within the method.
If you called round() with a different argument or assigned the method's result to a different variable, the result would not be what the method's header promises. Your code should always stick to the contract implied by the method header. If you can only understand what a method does by reading the implementation, that is bad object oriented design.[/QUOITE]
That makes sense. The method and header should match up.
The reason your code works is that you call round() using the argument tempValue and you do not assign the returned value. Change how you use round() and you get a bug.
OK, I think I understand now. Since I am not using the returned value, my program is not giving me any errors.
Now, I am trying to implement three comparison methods. I am not quite sure if I am doing it right, because when I test for equality, the two values are always equal. Maybe I am just not writing the driver (main) correctly. Here is my code:
Thanks,
Eric
What happens if "this" is a fahrenheit temperature?Originally posted by eric elysia:
Why are you setting the values when you're passed them into the constructor?
Why return 0 when the temperature scale asked for is the same as the temperature? Why is that an error?
How did you fix the round() function? Looking at the constructors, it seems the rounded value is ignored.
What happens if "this" is a fahrenheit temperature?
Why are you setting the values when you're passed them into the constructor?
Finally, since double's are approximate decimal values, you usually want to avoid testing with == but rather to see that the difference between them is "small enough" to consider them equal.
If both temperatures are using the same scale, no conversion is necessary. Otherwise, convert one of them to the other's scale (doesn't matter which one you pick). Your code assumes that the temperature receiving the method call is Celcius and the parameter passed in is in Fahrenheit.I was not sure how to compare the two temperatures.
You may find that these simple values can be represented exactly, but both the float and double types are binary approximations of decimal values (google for IEEE 754 for more information).Can I compare them for equality then? Should I use floats instead of doubles?
The reason I use round here is because when I create a new object (using new), I am intentionally using a tempValue parameter that has more than 1 digit after the decimal.The first and third constructors still have an error, but since you're using values that are already rounded, you don't see it.
This rounds tempValue (initialized to zero by the JVM) and then assigns the parameter initialTempValue to tempValue, overwriting the rounded 0.
I will need if statements to achieve this. The if statements will need to be in the comparison methods. I am just not sure how to write them.If both temperatures are using the same scale, no conversion is necessary. Otherwise, convert one of them to the other's scale (doesn't matter which one you pick). Your code assumes that the temperature receiving the method call is Celcius and the parameter passed in is in Fahrenheit.
So I shouldn't change the scale inside the method? Where is the best place to change the scale?Also, I just noticed that your getF() and getC() methods switch the temperature to the requested scale. This is what's called an unexpected side effect. Typically, getFoo() methods are expected not to change the object.
Since the double types are working in my equality tests, I will use them.You may find that these simple values can be represented exactly, but both the float and double types are binary approximations of decimal values (google for IEEE 754 for more information).
I understand that, but look at the constructors again. On the first line you round the instance variable tempValue which has not been assigned a value yet. On the second line you overwrite tempValue with the non-rounded parameter initialTempValue.Originally posted by eric elysia:
The reason I use round here is because when I create a new object (using new), I am intentionally using a tempValue parameter that has more than 1 digit after the decimal.
True, but they're very similar to the if statements you have in the getF() and getC() methods. Give it a try, post what you come up with, and I'll help you. I cannot simply give you the code.I will need if statements to achieve this. The if statements will need to be in the comparison methods. I am just not sure how to write them.
No, if you are going to change the scale, add a method like switchScale() or have setScale(scale) convert tempValue if the requested scale is different. But my the real question before doing that is: Do you need to change the scale of the TempTest object, or is it sufficient to calculate and return the converted value?So I shouldn't change the scale inside the method? Where is the best place to change the scale?
As you can see, this isn't rounded.
True, but they're very similar to the if statements you have in the getF() and getC() methods. Give it a try, post what you come up with, and I'll help you. I cannot simply give you the code.
No, if you are going to change the scale, add a method like switchScale() or have setScale(scale) convert tempValue if the requested scale is different. But my the real question before doing that is: Do you need to change the scale of the TempTest object, or is it sufficient to calculate and return the converted value?
I'm saying change neither value of the fields belonging to Temperature. Instead, convert a copy of tempValue and return it. To do this you'll need to add a second round() method that takes a temperature and returns it rounded to one decimal instead of rounding and returning tempValue.Originally posted by eric elysia:
If I am converting a temperature from F to C, then don't I want to change the F to C along with the actual temperature value? I am just trying to keep the two variables of the object together.
As another developer, I would expect getF() to return the value of the Temperature in degrees Fahrenheit without changing itself. I would expect convertToF() to modify the TempTest.
I'm saying change neither value of the fields belonging to Temperature. Instead, convert a copy of tempValue and return it. To do this you'll need to add a second round() method that takes a temperature and returns it rounded to one decimal instead of rounding and returning tempValue.
That's exactly what is happening. The other way would be to have the first constructor validate the units String before setting this.value to 0 and this.units to units. But as you can see, the second constructor is already doing that, so you're duplicating code. Having one constructor call another reuses the code you've already written. I just want to make the point that while it does save typing, it will help you avoid creating bugs in the future.It looks like Weight(String units) calls Weight(double value, String units). Is that correct?
You need a round() method that will take a parameter to be rounded instead of rounding tempValue. Here's psuedocode for getF() and convertF() as I see them.
Finally, regarding comparisons, it should be sufficient to get their values in the same scale (either F or C) and compare those. No need for if tests since getF() returns tempValue if tempScale is already F.
What I'm saying is that getF() should look as it did when you started.Now, forgetting about convertToF() entirely (pretend we never talked about it), change this method so it converts, rounds, and returns a copy of tempValue. It should not assign any values to tempValue or tempScale or call any other method that modifies them.Originally posted by eric elysia:
I don't understand how convertToF() will only convert temporaryTempScale.
I did say that originally, and you could write it that way.Another way to write it, though, is to choose one scale to use for comparison -- F or C -- and compare the temps using values in that single scale.By using getC/F() in the above methods and fixing getC/F() so they don't modify the Temperature itself, you will free yourself of unintended side effects.I don't understand why an if is not needed at all. I thought that is what you were saying that the method would need.
Java has an exception mechanism for just such errors, but you probably haven't covered it yet. Normally you'd throw an IllegalArgumentException with a nice message for the caller to handle. If the caller doesn't handle it, the JVM will exit when it catches it.Originally posted by eric elysia:
System.out.println("Error: Must be C or F.");
//This should take the user to...?
Either return without doing anything or call System.exit(1). Sorry, I forget that it's better not to give too many options at the early stages of learning, but I'm learning!Originally posted by eric elysia:
But since I haven't covered exceptions yet, should I call a method?
Don't get me started about those stupid light bulbs. |