• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Just curious how ugly this is?

 
Ranch Hand
Posts: 67
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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!



 
Sheriff
Posts: 14691
16
Eclipse IDE VI Editor Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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 ?)
 
Saloon Keeper
Posts: 15490
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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:
 
Sheriff
Posts: 22781
131
Eclipse IDE Spring VI Editor Chrome Java Windows
  • Likes 4
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

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.
 
Marshal
Posts: 79153
377
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Rob Spoor wrote: . . . Which will byte you in the behind . . .

Bad pun alert!
 
Rob Spoor
Sheriff
Posts: 22781
131
Eclipse IDE Spring VI Editor Chrome Java Windows
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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.
 
Bartender
Posts: 4568
9
  • Likes 2
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
When you're making bad programming puns without realising it, you know it's time to step away from the computer.
 
Christophe Verré
Sheriff
Posts: 14691
16
Eclipse IDE VI Editor Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Yeah. have a break;.
 
Stephan van Hulst
Saloon Keeper
Posts: 15490
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Posts: 67
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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?
 
Ranch Hand
Posts: 98
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Saloon Keeper
Posts: 15490
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I had some time to waste this afternoon, and this is what I came up with:
Filename: English.txt
Description: English numeral names
File size: 427 bytes
Filename: French.txt
Description: French numeral names
File size: 515 bytes
 
Ben David
Ranch Hand
Posts: 67
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Saloon Keeper
Posts: 15490
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Posts: 67
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
@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
Saloon Keeper
Posts: 15490
363
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

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
Posts: 98
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

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
Posts: 14691
16
Eclipse IDE VI Editor Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

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
Posts: 67
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks everyone!
 
Ben David
Ranch Hand
Posts: 67
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
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
Posts: 67
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

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.




 
Consider Paul's rocket mass heater.
reply
    Bookmark Topic Watch Topic
  • New Topic