• Post Reply Bookmark Topic Watch Topic
  • New Topic

Wanted some advise on following code (any logical mistakes or any code improvements are appreciated)  RSS feed

 
Abhishek Prajapati
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
[color=#444444]
[size=12]


[/size]

[/color]
 
Campbell Ritchie
Marshal
Posts: 56536
172
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Welcome to the Ranch

You have come to the right place; I shall make you suffer Mwaahaahaahaahaahaahaahaahaa!
Don't try mixing size colour and code tags; they don't work together. You seem to have got the code tags right, however But please write the question in the post, not only in its title.
Line 1: Why are you using that package name?
Line 6: What does the class name mean? It is not obvious from reading it.
Line 7: What do the two fields mean? It is not obvious from their names. Incorrect formatting: there shou‍ld be a space after the comma. Incorrect style; each field shou‍ld be declared on its own line.
Line 9: Intent of set method not obvious from its name. Design problem that you didn't write a constructor, so there is no guarantee that the fields will ever be set. Setting two fields in a setXXX method is inconsistent with the bean pattern, though it is not clear that you actually want to use the bean pattern.
Line 14: Another badly‑named method. If it returns a sum, why don't you call it sumOfSomething?
Line 19: Please explain why you have used suppress warnings. I know why, but you don't make it clear that you know why.
Why are you logging something when there is no Exception? Please explain what difference you have from System.out.println.
 
Abhishek Prajapati
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you so much Campbell Ritchie , and sorry for the weird post as of now its my first post and had no clue about tags and other stuff :P keeping
aside that answer for your questions are :

For Line 1 : I'd read in text that in java use of package is good practice and they use format "orgType.orgName.appName" as I'm not part of any organisation so just use com as package name.

Line 6 :  Actually I was trying to learn How to use Logger's info() method through this for displaying output so used LoggerEX (EX => as in example ) .

Line 7 : Okay got it ..that was bad idea to give them name as fs , they were actually variables for accepting users I/P for int values  .

Line 9 : hmm right its not code readable   actually i was thinking for constructor but as was trying to follow javaBean convention I left that part .

Line 19: I used @suppress as there was warning for Scanner's resources which I've used for input was not closed so I'd no clue how to handle that and then  with the help of eclipse I manged this ..

sorry but I don't have any presentable ans for that just one thing I was following advice from a article viz.
IBM Java Intro for beginers
in this article he has suggested to use Logging in spite of System.out.println(); so can you provide any ref for Logging where I can study this thing ..

 
Junilu Lacar
Sheriff
Posts: 11480
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Abhishek Prajapati wrote:I'm not part of any organisation so just use com as package name.

The reverse domain name is just a convention because domain names are unique values. The name you use here doesn't even have to be a real domain name. Some examples:

The only real requirement that package names impose is that the directory structure in which your .java source file is saved should match the package name structure. That is, given the above package names, the path to the .java source file for a program like LoggerEX would have to be:
 
Abhishek Prajapati
Greenhorn
Posts: 3
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
@Junilu Lacar hmm okay that makes sense, packaging is to avoid name collision in a working directory right. thanks so much for pointing that out
 
Campbell Ritchie
Marshal
Posts: 56536
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Abhishek Prajapati wrote:. . . Line 19: I used @suppress as there was warning for Scanner's resources which I've used for input was not closed so I'd no clue how to handle that and then  with the help of eclipse I manged this ..
. . .
I recognise the error message. Eclipse looks for resources opened and not closed, and mistakenly suggests you close a Scanner pointing to System.in. Fortunately you knew better than to believe Eclipse
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Abhishek Prajapati wrote:Line 19: I used @suppress as there was warning for Scanner's resources which I've used for input was not closed so I'd no clue how to handle that and then  with the help of eclipse I manged this ..

In which case, my advice would be:
1. Add a comment for duffers like me.
or
2. Don't suppress the warning.
It's possible that in future, Eclipse gets smart enough to work out that the "resource" is System.in; so the annotation is unnecessary.

Another annoying one is generic varargs, but I always use @SafeVarargs, rather than @SuppressWarnings("unchecked") to solve it, because it (hopefully) tells the person who's reading that I've done my homework. 

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