Junilu Lacar wrote:
As for the Double type, I need to see code that says Double is not appropriate, code that's in the form of a failing test.Michael Bruce Allen wrote:There is something really bothering me: Double
I do not like using this for the calculator, it seems too defined. I have heard of BigDecimal and BigInteger and thought about making a Number class. There needs to be an easy way to convert these to a string if needed as well. It should be able to take a number as large as I want. I do not know how to test for this though, since adding more too many numbers results in compiling error. This is where I am unsure.
Stephan van Hulst wrote:
@Before
public void setUp() {
this .calculation = new ReversePolishCalculation();
super.calculation = this.calculation;
}
// Tests specific to ReversePolishCalculation.
}[/code]
Junilu Lacar wrote:
...
Edit: Retrospecting on this a little bit, think about how the refactoring to extract a formula to the hasEnoughParameters() method allows you to realize that there is mis-assigned responsibility in your code. Would you have been able to see that otherwise? Does this show how good code leads to even better code? And how bad code can hide problems and that you need to refactor ruthlessly to eliminate bad code and bring out the goodness in the program?
In Aikido, this is part of shugyo
Junilu Lacar wrote:Next up, separation of concerns/responsibility. Obtaining the operands is a separate concern from using the operands. I'll separate these two a little bit:
...
The other operation classes would undergo the same transformation. After that, there is still a lot of duplication. This is where I see that a Binary operation is a generalization of all four operation classes. So I extract to a superclass:
...
There are even more opportunities to make this code cleaner by using some of the language constructs introduced in Java 8 but you can look into that later.
Junilu Lacar wrote:
Michael Bruce Allen wrote:Well, I did some searching around and learned of another way to do this. I like this way much better because it is more clear.
The thing is, you didn't really need to look anywhere outside for that. It was right there in the code. But I guess that recognition comes with experience.[/Quot]
Believe me man, I would have had to ask lots of questions before figuring this out. I didn't even really understand what an abstract method was until now.
Junilu Lacar wrote:
You took a distracting detour in making that method take a stack and two doubles. I guess that can happen though. I would have reverted when I saw how ugly it was instead of forging ahead from there. I'm trying to follow your commits and it seems you jumped the gun there somewhere and missed out on a few critical small steps in refactoring. The end result is almost the same but in this case, the means is just as important as the end.
...
I immediately see a pattern repeated in this code. A pattern indicates that there is an opportunity to generalize by extracting similarities and parameterizing differences. The first thing I see is that formula, stack.size() > 1.
...
I ask "What does that formula mean? What is the essence of it?" -- Answer: "I need two operands to perform this operation, so I'm checking that there are at least two operands available to pop from the stack."
https://github.com/CodeAmend/calculator/blob/temp/src/calculator/functions/basic/OperandStack.java
Within my temp branch the other day, I did this very thing. I called it stackHasEnough() lol
I remembered doing this before, but I felt I didnt like the code. One thing I did not realize is the importance of code clarity. I thought I understood, but because I wanted to change this, I never refactored it (this time), If I would have just renamed it, I might have gotten a better idea.