aspose file tools*
The moose likes Beginning Java and the fly likes Just curious how ugly this is? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Java » Beginning Java
Bookmark "Just curious how ugly this is?" Watch "Just curious how ugly this is?" New topic
Author

Just curious how ugly this is?

Ben David
Ranch Hand

Joined: Oct 01, 2010
Posts: 67
I am trying to get better at writing code more like a programmer and less like a scripter. This is the first recursive method I have written for a problem. The program just converts a double into a string as a dollar and cents value written entirely in text.

I am just curious how "ugly" or "pretty" this code looks? What could I have improved upon, or done better?

I realize this is a really open ended question, and a large section of code.

So I understand if nobody has time to respond to this. Thanks anyway!



Christophe Verré
Sheriff

Joined: Nov 24, 2005
Posts: 14687
    
  16

It looks like you could use some arrays to hold the strings ({"one", "two"...}) and get rid of most of the big switch/case blocks.

(What is the "throw new RuntimeException();" doing line 91 ?)


[My Blog]
All roads lead to JavaRanch
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3647
    
  16

As far as simple aesthetics go, I prefer to put braces right behind the condition / method signature. I also prefer to leave them out completely if the conditional statement is simple. For simple switch cases, I prefer to put the statement right after the case. I edited one of your methods to show how I usually format my code:
Rob Spoor
Sheriff

Joined: Oct 27, 2005
Posts: 19697
    
  20

Stephan van Hulst wrote:I also prefer to leave them out completely if the conditional statement is simple.

Which will byte you in the behind when you want to add a debugging line temporarily and forget to add the braces at that time:
That's why I always use brackets in if, for, while etc statements.

You know, Python solved this issue by requiring all statements in the same block to have the same indentation. It also forces you to keep your code properly formatted.


SCJP 1.4 - SCJP 6 - SCWCD 5 - OCEEJBD 6
How To Ask Questions How To Answer Questions
Campbell Ritchie
Sheriff

Joined: Oct 13, 2005
Posts: 38902
    
  23
Rob Spoor wrote: . . . Which will byte you in the behind . . .
Bad pun alert!
Rob Spoor
Sheriff

Joined: Oct 27, 2005
Posts: 19697
    
  20

More like a typo alert. I actually meant "bite", not "byte". My mind must have been somewhere else, or still moving from one topic to this one.
Matthew Brown
Bartender

Joined: Apr 06, 2010
Posts: 4392
    
    8

When you're making bad programming puns without realising it, you know it's time to step away from the computer.
Christophe Verré
Sheriff

Joined: Nov 24, 2005
Posts: 14687
    
  16

Yeah. have a break;.
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3647
    
  16

Rob, there was a time I would agree with you completely, but I've just experienced that using less braces and making my code more compact makes it much easier for me to read it. This is far more important to me than the small amount of time I spend fixing the bug when I mess up.

I also like to use an empty line around blocks/conditional statements, so it makes it easy for me to see that I have to add braces when I add another statement.

It's just a personal preference.

// On the other hand, your comment about Python, that just seems terrible to me :P
// I think there should never be any restrictions on the amount of whitespace used. Forcing a programming to use proper indentation should be an optional feature of the IDE, not of the language.
Ben David
Ranch Hand

Joined: Oct 01, 2010
Posts: 67
Thanks for all the responses!!

Though I am afraid I think I was terribly unclear! It was a poor choice of words on my part to use the words "ugly" and "pretty".

I was not actually wondering about aesthetics at all, (though I did actually like those critiques as well, thanks!) rather I am more interested in hearing critique about the meat of the program.


Other than reducing many of the switch statements to Arrays (Thanks for the idea Christophe!), is there anything else I could have done smarter or more efficiently?

Are there places where I seem to have gone around my elbow to get to my....

Would a completely different approach have made for a simpler, more concise solution?

Have I done anything that is outside programming best practices?


Basically, being someone who has done more scripting that programming, I feel I have a tendency to write more spaghetti like code. How would you rate this code for efficiency, and non-wastefulness.
(by efficient/wastefulness I don't just mean as far as system resources at runtime -- but also as far as the human writing/reading of the code)

p.s. @Christophe -- the exceptions are just thrown in places, that I never wanted to reach in the code, and where eclipse was requesting I put a return value. So I just threw a generic exception, so I would know where to investigate. Is this a bad use of that? Is there a better approach I could have taken to the situation?
Kurt Van Etten
Ranch Hand

Joined: Sep 07, 2010
Posts: 98
Hi Ben,

Your program is quite similar to one of the assignments on the cattle drive here. Take a look at assignment Java-4b. There are style guidelines there also, and you can browse through the older threads to see some discussion of the assignments. But you'll need to sign up for the cattle drive to get the full low-down. The bottom line, though, is that you might be surprised at just how concise you can make the code for this task.
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3647
    
  16

I had some time to waste this afternoon, and this is what I came up with:


[Download English.txt] Download

[Download French.txt] Download

Ben David
Ranch Hand

Joined: Oct 01, 2010
Posts: 67
Wow -- Thanks! I want to get this running in eclipse so I can watch it work. (quickest way for me to fully understand it probably)

How are you getting the values from english.txt into the Map?

And would I just call "string = convert(decimal)" from a main method to use this?
Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3647
    
  16

Before you can use the convert method, you have to construct a new DecimalText instance, using a Scanner to provide the name mapping.

Take a look at java.util.Scanner. It should have just the constructor you need.

[edit]

Note that the Scanner class may also be useful for you to test whether input Strings can safely be converted to BigDecimal.
Ben David
Ranch Hand

Joined: Oct 01, 2010
Posts: 67
@Stephan:

Thanks for taking the time to write that!! If you dont have time to respond to all this, I understand!

I took and look and understand how this works now. Very simple and elegant. Exactly how I want to be able to program! I seem to always make things harder than they need to be.

I am guessing there is probably no easy thing you can tell someone about how to approach a problem to come up with elegant algorithms like this, but just curious if there are pointers you can give.
How to originally approach/think about a the problem.
How to tell when you are making it harder than it needs to be, etc.
Are there any books you might recommend that teach this sort of thing.

The only problem I found with this, is that if there is one hundred or one thousand, etc -- it leaves out the "one" part, and just puts "hundred".

Below is my fix for this. I am just curious. Would you have had an even simpler more elegant fix?



And to make this do the full thing of saying integer, as well as the first two decimal places, I would probably, in the main method, just call it on the decimal, then subtract the rounded down decimal from itself (to get just the decimal part) and multiply by 100, and then give that the convert to get the two digits after the decimal.
(And also just add the "dollars" and "cents" in the main method. Is that probably the easiest way, or would you have a more elegant solution for that up your sleeve?

Stephan van Hulst
Bartender

Joined: Sep 20, 2010
Posts: 3647
    
  16

Ben David wrote:I am guessing there is probably no easy thing you can tell someone about how to approach a problem to come up with elegant algorithms like this, but just curious if there are pointers you can give.
How to originally approach/think about a the problem.
How to tell when you are making it harder than it needs to be, etc.
Are there any books you might recommend that teach this sort of thing.


Well I usually think about the problem "on paper". Never delve into the coding right away. What I did for this one was write down a random number, and then I analyzed for myself how we convert these numbers to speech, in our heads.
We parse the number from left to right, counting the times we can fit powers of ten in there. Then I realized that some languages don't even use powers of ten all the time, that's why I added the French text.
Then to represent the count, I figured I could use a simple recursion. You will be surprised by how much more compact some programs can be made by recursive functions.

In general, I would recommend writing down some sample problems on paper, and then solve them by hand. Doing a few cases for yourself, without the use of a computer, will often drastically improve the insight you have in the general case.

A good indication that you're "making things harder" is when your code is very specific. Using switch statements is often (not always) a good indication. Switch statements are used to deal with specific cases. Like Christophe said, you can represent most cases with an array, and then use for loops. In my program, I used a file/map instead of an array. The power of a computer language is that it provides while/do/for loops. Use them.

I can't really think of any books that deal with this subject. I'm sure they're there, but I haven't read them. One thing that I know helps a lot is practice. Practice practice practice. When you see a small problem that looks like fun, just write a program to solve it. I once made a program to draw names out of a hat for Christmas presents. Using an actual hat we kept drawing our own names.

The only problem I found with this, is that if there is one hundred or one thousand, etc -- it leaves out the "one" part, and just puts "hundred".
Below is my fix for this. I am just curious. Would you have had an even simpler more elegant fix?


Your solution is just fine. The only thing is that it sacrifices language independence (which isn't a big problem if you only care about one language). You may consider assuming that the first line in the file *always* represents the unit, and you can store that term and add it to the string instead of the literal "one-". In this case you can edit the entire program to use BigInteger instead of BigDecimal.

Another thing to consider is saving a BigDecimal constant HUNDRED, instead of using valueOf(100) each time. It's not a big deal though. Note that hardcoding this value again breaks language independence. What you could do is edit the language files to indicate which terms need to be preceded by the unit term, if the count is 1.

And to make this do the full thing of saying integer, as well as the first two decimal places, I would probably, in the main method, just call it on the decimal, then subtract the rounded down decimal from itself (to get just the decimal part) and multiply by 100, and then give that the convert to get the two digits after the decimal.
(And also just add the "dollars" and "cents" in the main method. Is that probably the easiest way, or would you have a more elegant solution for that up your sleeve?


Nope. That's exactly how I would do it. I considered adding a main method to it, but I thought I'd let you figure it out ;)
Kurt Van Etten
Ranch Hand

Joined: Sep 07, 2010
Posts: 98
Ben David wrote:Are there any books you might recommend that teach this sort of thing.

I asked a somewhat related question over on this thread, and was recommended these books:

Code Complete 2 by Steve McConnell
Clean Code by Robert Martin

I don't know how well those might address your interests, but they're both searchable on Amazon so you could browse through them.
Christophe Verré
Sheriff

Joined: Nov 24, 2005
Posts: 14687
    
  16

p.s. @Christophe -- the exceptions are just thrown in places, that I never wanted to reach in the code, and where eclipse was requesting I put a return value. So I just threw a generic exception, so I would know where to investigate. Is this a bad use of that? Is there a better approach I could have taken to the situation?

You don't need to return anything for void methods, so you can remove the throw statements of these methods. For other methods, those returning values, it depends. Sometimes it makes sense to return an empty value, like an empty String, or an empty Collection. Sometimes, it makes sense to throw something like an IllegalArgumentException. A plain RuntimeException is too broad and discouraged.
Ben David
Ranch Hand

Joined: Oct 01, 2010
Posts: 67
Thanks everyone!
Ben David
Ranch Hand

Joined: Oct 01, 2010
Posts: 67
So I wanted to make this use spaces instead of dashes, but use dashes for seventy-five, or thirty-four, etc.

just curious if this is a good way to do it, or if there is an even cleaner way.



And before the for loop I declare
Ben David
Ranch Hand

Joined: Oct 01, 2010
Posts: 67

Also -- I just realized a much cleaner way to get the "one" in front of the hundred or thousand, etc. I dont know why I did not think of it before. Really, it should have just been sent to the else block along with all the other hundreds (two, three, etc.) instead of handling it as a special case in the if count == 1 block. So instead of testing count ==1, I just test that the key is < 100.




 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
 
subject: Just curious how ugly this is?