• Post Reply Bookmark Topic Watch Topic
  • New Topic

Advice on my programming technique  RSS feed

 
Greenhorn
Posts: 4
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi everyone. I'm a computer science student and I'm learning java on the sidelines (we're doing Python in my current course). I was hoping that some of you really experienced programmers can help me learn some new stuff or just give me advice on my programming in general. So I'm going to post some of my code once in a while and I welcome any and all critique.

Okay so this is something I wrote that converts a given integer between 1 and 3999 to roman numerals (I had originally written it in Python). Any improvement/errors or anything else that I made I really want you to point out. Oh and I'm new to the forum so forgive me if this is in the wrong place or something. Oh and the code works but it feels somewhat bloated haha. Thanks
 
Marshal
Posts: 56600
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch

I was about to add code tags to your post, and you can see how much better it looks, only somebody else beat me to it!

Start by getting rid of the GUI code. You can put the GUI code back when you have everything else working. you should have a class with a method which converts Arabic (so called because it wasn't invented in Arabia ‍) notation to Roman. I think the conversion looks like what the mathematicians call a function, so it can be a static method in a utility class. You should create a RomanAndArabic class, which can have two methods.
Move 99% of that code out of the main method. A main method comprising more than one statement is incorrect. (The FAQ shows two lines but only one of them is actually a statement.)

That should give you enough to keep you busy for the rest of today
 
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Albert Mitton wrote:Hi everyone. I'm a computer science student and I'm learning java on the sidelines (we're doing Python in my current course). I was hoping that some of you really experienced programmers can help me learn some new stuff or just give me advice on my programming in general. So I'm going to post some of my code once in a while and I welcome any and all critique.

Well, first up: Thank God I'm not the only one to have created a crn() (convert to roman numerals) script. That alone deserves a +1.

Cited in an ancient - and anal-retentive - text (which unfortunately, I can't find any more) as "the most useless reason for programming ever devised", it nevertheless taught me a LOT about scripting, so I'm glad I ignored it.

However (and this would be my main criticism): It is coded precisely like a script - very few variables, no methods, no normalization. It's simply a great lump of code.

I suspect it probably does the job, but it's not a pattern you could reproduce for more complex problems.

And I suspect that an experienced Python programmer would have told you the same.

My 2¢

Winston
 
Albert Mitton
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie wrote:Welcome to the Ranch

I was about to add code tags to your post, and you can see how much better it looks, only somebody else beat me to it!

Start by getting rid of the GUI code. You can put the GUI code back when you have everything else working. You should have a class with a method which converts Arabic (so called because it wasn't invented in Arabia ‍) notation to Roman. I think the conversion looks like what the mathematicians call a function, so it can be a static method in a utility class. You should create a RomanAndArabic class, which can have two methods.
Move 99% of that code out of the main method. A main method comprising more than one statement is incorrect. (The FAQ shows two lines but only one of them is actually a statement.)

That should give you enough to keep you busy for the rest of today


Thanks for the feedback! I will work on the things you mentioned.
 
Albert Mitton
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:
I suspect it probably does the job, but it's not a pattern you could reproduce for more complex problems.

And I suspect that an experienced Python programmer would have told you the same.



Thanks for the feedback . How do you suggest I can change it to be more of a pattern or to use patterns? Also If it sounded as if I'm an experienced Python programmer, I'm really not :p. I'm only in my first year and only started learning Python about 2 months ago.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Albert Mitton wrote:Thanks for the feedback . How do you suggest I can change it to be more of a pattern or to use patterns?

Well, the first thing would probably be to use a Map (or arrays or Lists) to store your "factors" along with their "symbol", eg:

private int[] factors = new int[] {1, 5, 10, 50, 100, 500, 1000};
private char[] symbols = new char[] {'I', 'V', 'X', 'L', 'C', 'D', 'M'};


or possibly have your factors work cumulatively, eg:
private int[] factors = new int[] {5, 2, 5, 2, 5, 2};
Do you see how that might help you?

But the main thing right now is to StopCoding (←click) and write down the steps needed to convert a decimal number to Roman numerals in English (or your native language). And once you have that, I'd suggest making each "step" a method.

And while you're at it, you might want to think about those "one before" situations. You can write "IV", "IX", "XL", "XC", "CD" and "CM", but I've never seen "IC". Why might that be?
Of course, the problem is small enough to just do it by brute force, but why not come up with some "rules"? Then you could extend the calculator, say with 'v', 'x', 'l', 'c', 'd', and 'm' to represent multiples of thousands up to a million, then another six symbols to go to a billion... and so on.

But the main thing to do is think, not just bash out code.

HIH

Winston
 
Albert Mitton
Greenhorn
Posts: 4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:
Albert Mitton wrote:Thanks for the feedback . How do you suggest I can change it to be more of a pattern or to use patterns?

Well, the first thing would probably be to use a Map (or arrays or Lists) to store your "factors" along with their "symbol", eg:

private int[] factors = new int[] {1, 5, 10, 50, 100, 500, 1000};
private char[] symbols = new char[] {'I', 'V', 'X', 'L', 'C', 'D', 'M'};


or possibly have your factors work cumulatively, eg:
private int[] factors = new int[] {5, 2, 5, 2, 5, 2};
Do you see how that might help you?

But the main thing right now is to StopCoding (←click) and write down the steps needed to convert a decimal number to Roman numerals in English (or your native language). And once you have that, I'd suggest making each "step" a method.

And while you're at it, you might want to think about those "one before" situations. You can write "IV", "IX", "XL", "XC", "CD" and "CM", but I've never seen "IC". Why might that be?
Of course, the problem is small enough to just do it by brute force, but why not come up with some "rules"? Then you could extend the calculator, say with 'v', 'x', 'l', 'c', 'd', and 'm' to represent multiples of thousands up to a million, then another six symbols to go to a billion... and so on.

But the main thing to do is think, not just bash out code.

HIH

Winston


Thank you, I'll have a go at doing that. I've spent most of my day so far going over java reading material and I've already been able to change some of the things you guys have pointed out like not using the main method etc. I really appreciate the help .

Albert
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!