running away from rain and falling in water

[miguel lisboa]

i found this tutorial, quite similar to test infected one.
i followed it till a certain point, but now i'm stuck
when author says:

We need to create methods to make the test compile, watch it fail, then make it pass, step by step. As the textbooks say, we leave this as an exercise to the reader.

i cant imagine how to o from here
before i show what i've so far, let me say i cant imagine what convertAtRate() body should look like (maybe empty?!)
as to getValue(string) i just cant write it, at least using just the refered methods in tests

 package infected;
 
public class Money {
private int value;
private String currencySymbol;
 
public Money(final int aValue) {
value = aValue;
}
 
public Money() {
}
 
public Money(int aValue, String aCurrencySymbol) {
value = aValue;
currencySymbol = aCurrencySymbol;
}
 
public void add(final Money aMoney) {
value += aMoney.getValue(); // <-- Simply add the values
}
 
public int getValue() {
return value;
}
 
public String getCurrencySymbol() {
if (currencySymbol != null) {
return currencySymbol;
}
else
return "CAD";
}
static String currencySymbol1, currencySymbol2;
static double rate;
public static void convertAtRate(String currencySymbola,
String currencySymbolb, double aValue) {
// Money.convertAtRate("USD", "CAD", 1.58);
/*
 * us/cad=1.58 us=1.58 * cad
 */
currencySymbol1 = currencySymbola;
currencySymbol1 = currencySymbolb;
rate = aValue;
}

int a1, a2;
 
public int getValue(String currencySymbol) {
Money m1 = new Money(a1, currencySymbol1);
Money m2 = new Money(a2, currencySymbol2);
 
convertAtRate(currencySymbol1, currencySymbol2, rate);
 
/*
 * Money.convertAtRate("USD", "CAD", 1.58); 
 * final Money money = new Money(100, "USD"); 
 * assertEquals(158, money.getValue("CAD"));
 */
a2 = (int) (a1 * rate);
System.out.println("rate: " + rate + " " + "\na1: " + a1);
return a2;
}
}
 
package infected;
 
import junit.framework.TestCase;
 
public class MoneyTestCase extends TestCase {
 
public void testAddSameCurrency() {
// STEP 1: Create some objects
final Money money1 = new Money(20);
final Money money2 = new Money(30);
 
// STEP 2: Send some messages
money1.add(money2);
 
// STEP 3: Verify the results.
assertEquals(50, money1.getValue());
assertEquals(30, money2.getValue());
 
}

public void testCreateDefaultCurrency() {
    assertEquals("CAD", new Money().getCurrencySymbol());
}

public void testCreate() {
    final Money money = new Money(60, "USD");
 
    assertEquals(60, money.getValue());
    assertEquals("USD", money.getCurrencySymbol());
}
 
public void testConvert() {
    Money.convertAtRate("USD", "CAD", 1.58);
    final Money money = new Money(100, "USD");
 
    assertEquals(158, money.getValue("CAD"));
    assertEquals(100, money.getValue("USD"));
    assertEquals(100, money.getValue());
    assertEquals("USD", money.getCurrencySymbol());
}
 
}
 

maybe someone (very patient) may help me
thanks in advance

[Jeanne Boyarsky]

Miguel,
Note that convertAtRate doesn't return anything. Therefore, we are testing its side effects (in other words, it sets something.) Since you are telling it the conversion rate between two currencies, convertAtRate should store that somewhere in the object. I'll leave the data structure as an exercise to you , but do let us know if you are stuck.

[miguel lisboa]

thanks a lot for your help!
well, so far so good, but, if possible, i'd like critic about my aproach

 package infected;
 
public class Money {

private int value;
private String currencySymbol;
 
public Money(final int aValue) {
value = aValue;
}
 
public Money() {
}
 
public Money(int aValue, String aCurrencySymbol) {
value = aValue;
currencySymbol = aCurrencySymbol;
}
 
public void add(final Money aMoney) {
value += aMoney.getValue(); // <-- Simply add the values
}
 
public int getValue() {
return value;
}
 
public String getCurrencySymbol() {
if (currencySymbol != null) {
return currencySymbol;
}
else
return "CAD";
}
 
static double rate;

public static void convertAtRate(String currencySymbola,
String currencySymbolb, double aValue) {
rate = aValue;
}
 
public int getValue(String currencySymbol) {
 
if (currencySymbol.equals("CAD")) {
return (int) (getValue() * rate);
}
else
return (int) (getValue());
}
}
 

[ March 10, 2005: Message edited by: miguel lisboa ]

[Jeanne Boyarsky]

Miguel,
I'm not sure if you are looking for any specific comments, but here's what I would say if doing a code review:

1) I'm not sure why "CAD" is hard coded in the class. If it is the default, I would make it a constant with a name that explains the purpose.
2) Is it valid to call convertAtRate() multiple times? Should the program remember the different rates of just the last ones?
3) It looks like the methods/fields are in the order you typed them in. There is nothing forcing you to only enter code at the bottom. In particular, it was hard to find the rate field because it was between methods.
4) Not sure what a1 and a2 do. You should consider some more descriptive names.
5) Is the first constructor (the one that takes an int) still valid? If so, what currency is used?
6) I'm not so comfortable with "return (int) (getValue() * rate);". Does it truncate amounts? If I convert $10 with a rate of 9.99, do I lose money?

I realize that some of these are from the tutorial, but these would be my comments independent of it.

The one thing I don't like about code reviews is we (myself included) rarely remember to mention all the things that are good about the code. In particular, I like how you used the final keyword. And the code definitely looks TDD'd.

[miguel lisboa]

I'm not sure if you are looking for any specific comments, but here's what I would say if doing a code review

i wish i could allways have it

1) I'm not sure why "CAD" is hard coded in the class. If it is the default, I would make it a constant with a name that explains the purpose.

great!

2) Is it valid to call convertAtRate() multiple times?

i'm not sure about what you mean...

2)(...)Should the program remember the different rates of just the last ones?

just the last ones?!

3) It looks like the methods/fields are in the order you typed them in. There is nothing forcing you to only enter code at the bottom. In particular, it was hard to find the rate field because it was between methods.

well, here i think i'm not sure about what i'm doing
some time ago i did like you say and then they criticized me, saying looked like cobol, others with the argument that's easier to find var declarations; you may not believe, but i tend to prefer declare them all at the start

4) Not sure what a1 and a2 do. You should consider some more descriptive names.

that's true: i rushed to post and only afterwards i refactored properly and deleted both

5) Is the first constructor (the one that takes an int) still valid? If so, what currency is used?
6) I'm not so comfortable with "return (int) (getValue() * rate);". Does it truncate amounts? If I convert $10 with a rate of 9.99, do I lose money?

yes, blame authors ...

I like how you used the final keyword.

i dont want another's credits.
at forums people often refer that using and IDE is bad for learning; as what i'm concerned i feel very confortable with eclise.
I tried to write "static" but eclipse said no, only final...

And the code definitely looks TDD'd.

how can you tell?

Let me tell you how gratefull i am with your post!
thanks a lot
btw i'm stuck again, later in this very tut; as soon i feel ready i'll post what i've got
thanks a lot!


edited:
what was stucking me was a typo: CAN instead of CAD
here's my new code:

 package infected;
 
public class Money {
 
private int value;
private String currencySymbol;
static double rate;
 
public Money(final int aValue) {
value = aValue;
}
 
public Money() {
}
 
public Money(int aValue, String aCurrencySymbol) {
value = aValue;
currencySymbol = aCurrencySymbol;
}
 
public void add(final Money aMoney) {
Money m = aMoney;
if (getCurrencySymbol().equals(m.getCurrencySymbol())) {
value += m.getValue();
}
if (m.getCurrencySymbol().equals("CAD")
&& getCurrencySymbol().equals("USD")) {
value = (int) (m.getValue() / rate) + getValue();
}
}
 
public int getValue() {
return value;
}
 
public String getCurrencySymbol() {
if (currencySymbol != null) {
return currencySymbol;
}
else
return "CAD";
}
 
public static void convertAtRate(String currency1, String currency2,
double aValue) {
rate = aValue;
}
 
public int getValue(String currency) {
 
if (currency.equals("CAD")) {
return (int) (getValue() * rate);
}
 
if (getCurrencySymbol().equals("CAD") && currency.equals("USD")) {
return (int) (getValue() / rate);
}
else
return (int) (getValue());
}
}
 

dont want to abuse, but pls help me with your point 1) - cant figure it out: why?
thanks a lot again

[ March 12, 2005: Message edited by: miguel lisboa ]
[ March 12, 2005: Message edited by: miguel lisboa ]

[Jeanne Boyarsky]

Miguel,
1) If you are using an IDE, you can do an "extract constants" refactoring on it.

2) What should happen if I run the following code?

Money.convertAtRate("USD", "EUR", 2);
Money.convertAtRate("USD", "CAD", 1.5);
Money money = new Money(5, "USD");
money.getValue("EUR");

Logically I would expect it to convert to euros. The current implementation would not longer have the exchange rate though. I guess I'm asking whether the program is meant to only support two currencies at a time? (USD and EUR) In this case CAD is the last currency I was referring to.

3) I've seen a lot of debate of where the field declarations should be within a method. I haven't seen any over where the instance field declarations should be in a class. Granted if you are using an IDE, this isn't terribly relevant. I prefer to declare them all at the start too. That was what I was trying to say. Rate isn't declared at the start.

4) Great!

5/6) Sure, why not...

You can tell code was TDD'd because the methods are short, simple and to the point. Also, there is no extra/unneeded code in TDD'd code.

New commentary:
Why isn't rate a "private static" variable? In other words, why isn't it private?

[miguel lisboa]

Why isn't rate a "private static" variable? In other words, why isn't it private?

because once again i was in a rush:
private double rate; (i dont see the need of making it static)
thanks a lot, really!

[miguel lisboa]

private double rate; (i dont see the need of making it static)

its a mistake: rate is referenced from a static context so must be:

private static double rate;  

[Ilja Preuss]

Originally posted by Jeanne Boyarsky:
2) What should happen if I run the following code?

Money.convertAtRate("USD", "EUR", 2);
Money.convertAtRate("USD", "CAD", 1.5);
Money money = new Money(5, "USD");
money.getValue("EUR");

Logically I would expect it to convert to euros. The current implementation would not longer have the exchange rate though. I guess I'm asking whether the program is meant to only support two currencies at a time? (USD and EUR) In this case CAD is the last currency I was referring to.

I think that test is missing from the tutorial...