Win a copy of Programmer's Guide to Java SE 8 Oracle Certified Associate (OCA) this week in the OCAJP forum!
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Simple Class, how does this look?

 
John Logan
Greenhorn
Posts: 6
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I've created a little program to print out all factors of a certain number (supplied by the command line). It seems to work pretty nice and helps me refresh my memory of math while I'm learning Java
Could I have some opinions on the code, telling me if I've done something wrong/not well/inefficient etc? It's the first time I've truly "rolled my own" as my previous code has really been out of books - so be gentle please

Obviously I run this by java Factors [number] from the command line.
Thanks all.
[ August 11, 2002: Message edited by: John Logan ]
 
Michael Morris
Ranch Hand
Posts: 3451
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi John,
Well for a first attempt at writing a class it ain't bad. Here are some suggestions:
  • The javadoc comment for your class should occur before not inside the class definition
  • For your constructor you used a block comment instead of a javadoc comment
  • You left your num variable at package scope. It should probably be private.
  • There may be more efficient ways to factor an integer using recursion, but that's a judgement call. I discourage recursion except in known algorithms.
  • You probably shouldn't write to stdout except in your main method. Think about returning an array of ints and writing them out in main.


  • Hope this helps,
    Michael Morris
     
    John Logan
    Greenhorn
    Posts: 6
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Michael,
    Thanks for the comments info, I wasn't sure about that.
    # You probably shouldn't write to stdout except in your main method. Think about returning an array of ints and writing them out in main.

    That's great, I'll work on trying this out now.
    Cheers,
     
    Barkat Mardhani
    Ranch Hand
    Posts: 787
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi John:
    What if a zero, negative integer or non-integer is provided at command line....
    Barkat
     
    Michael Morris
    Ranch Hand
    Posts: 3451
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Good point Barkat. We should throw some sort of Exception, shouldn't we?

    Michael Morris
     
    John Logan
    Greenhorn
    Posts: 6
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi guys,
    I think exceptions/error catching is a bit beyond me atm, but it's something I'll be hoping to put in a later time when I know a bit more about them
    Thanks for the suggestion.
    John
     
    Anthony Villanueva
    Ranch Hand
    Posts: 1055
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    You can try the exceptions tutorial from Sun.
     
    Marilyn de Queiroz
    Sheriff
    Posts: 9066
    12
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Or -- You can test
    if number > 0, continue
    and if not, print "number must be greater than zero".
     
    Ilja Preuss
    author
    Sheriff
    Posts: 14112
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Originally posted by Michael Morris:
    You probably shouldn't write to stdout except in your main method. Think about returning an array of ints and writing them out in main.

    I agree with that - letting it write directly to stdout violates the Single Responsibility Rule and decreases reusability.
    Even better than returning an array might be to implement it iterator-like:
     
    Michael Morris
    Ranch Hand
    Posts: 3451
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Originally posted by Ilja Preuss:
    Even better than returning an array might be to implement it iterator-like

    No doubt a more elegant solution. But then we should probably store our results in a Set as well to make iteration very simple to implement.
    Michael Morris
     
    Ilja Preuss
    author
    Sheriff
    Posts: 14112
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Originally posted by Michael Morris:
    No doubt a more elegant solution. But then we should probably store our results in a Set as well to make iteration very simple to implement.

    That certainly would be one way. I wonder how hard it might be if we simply remembered the next factor to return, though...
     
    • Post Reply
    • Bookmark Topic Watch Topic
    • New Topic