Win a copy of Real-World Software Development: A Project-Driven Guide to Fundamentals in Java this week in the Agile and Other Processes forum!
  • 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 all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Paul Clapham
  • Liutauras Vilda
  • Knute Snortum
  • Bear Bibeault
Sheriffs:
  • Devaka Cooray
  • Jeanne Boyarsky
  • Junilu Lacar
Saloon Keepers:
  • Ron McLeod
  • Stephan van Hulst
  • Tim Moores
  • Carey Brown
  • salvin francis
Bartenders:
  • Tim Holloway
  • Piet Souris
  • Frits Walraven

Basic questions

 
Ranch Foreman
Posts: 74
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello! I wrote a little console calculator application. Would you suggest any improvements?
(1) The problem is that Eclipse saying for example for every System.out "Replace this use of System.out or System.err by a logger."
How I can replace it with logger and why is it better?
(2) Then eclipse says that I never use Object "input" - but I think I use it every loop again, so where is the problem?
(3) And third it says for "do" expression: "Reduce the total number of break and continue statements in this loop to use at most one." - I use breaks in switch cases, so why reduce?
Thank you!


 
Marshal
Posts: 68044
258
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Mike Savvy wrote:. . . . Would you suggest any improvements?

Yes. Get all that code out of the main() method. You aren't writing object‑oriented code. Where have you got a Calculator object, for example?
Don't use System.exit(). Arrange for your loop to terminate when you write, “stop”.

. . . "Replace this use of System.out or System.err by a logger."

Eclipse makes suggestions, not instructions. That may be one of several suggestions for the same code. Many people use loggers for exception messages so they have the information in a file and can correct anything in the code. The stack trace would be incomprehensible to end‑users.

. . . "Reduce the total number of break and continue statements . . ." . . .

You have written continue; twice.
The structure of that loop isn't at all good; it is an infinite loop with only System.exit() to escape it.
Nor am I happy with your exception handling. You shouldn't be doing argument validation in that loop. The keyboard input and validation belong elsewhere, probably in a utility class.
Don't mix Scanners and Double.parseDouble(). The Scanner has enough methods to get the input directly without your needing to use parseXXX() methods. What's more, the hasNextXXX() methods allow you to avoid using exceptions at all.
 
Bartender
Posts: 21720
148
Android Eclipse IDE Tomcat Server Redhat Java Linux
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
System.in, System.out, and System.err are the unholy trinity of Unix-style programs. There is absolutely nothing wrong with using them for command-line applications. That's what they were designed for, in fact.

However, not all environments are designed to work that way. Web applications, for example. For cases like that, a logger is much better because A) the System I/O classes don't officially exist in the web application paradigm and B) you get more functionality out of loggers.

A logger can do many things that simple stdio cannot. You can set logging levels on your log statements so that fine details will not ordinarily be output, but with the "flip of a switch" you can turn them on without having to make extensive program changes. This is invaluable when a program is running in production and it starts acting up.

Another thing loggers can do is routing. You can select the destination - if any - for different components of an application based on its logger ID, which is often based on the package structure of the app and its libraries (if you're using third-party libraries). For example, by setting the logging parameters correctly, you can see the SQL that Hibernate is generating for your application if you're using Hibernate. You can also do things like email and/or page people if a high-severity log message is generated such as "disk nearly full". You can log to a database or a central log manager for archival purposes.

And then there are other useful things. Like logging usually adds a timestamp and a subsystem identifier to the log messages to make it automatically more obvious when and where something happened. And you can log Exceptions with their stack traces using simple code.

You don't need (or even want!) any of this for a simple app like "input 2 numbers and output their sum", but logging is very definitely useful in many industrial-grade systems.
 
Saloon Keeper
Posts: 2511
117
Google Web Toolkit Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Mike Savvy wrote:...
(1) The problem is that Eclipse saying for example for every System.out "Replace this use of System.out or System.err by a logger."


I'd like to add that this is not a default feature in Eclipse. Rather, I think your eclipse installation has a plugin like SonarLint which does a code quality check and suggests the above action

Mike Savvy wrote:
(2) Then eclipse says that I never use Object "input" - but I think I use it every loop again, so where is the problem?


I think that the warning message is "Resource leak: 'input' is never closed". You can ignore this warning.
 
Campbell Ritchie
Marshal
Posts: 68044
258
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

salvin francis wrote:. . . "Resource leak: 'input' is never closed". You can ignore this warning.

Eclipse gives you that warning, which I have seen many times, whenever you fail to close a Scanner or similar. You mustn't close a Scanner reading System.in, otherwise you close System.in. The correct option to pick is that with @SuppressWarnings....

You must however close a Scanner reading everything else.
 
salvin francis
Saloon Keeper
Posts: 2511
117
Google Web Toolkit Eclipse IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Mike Savvy wrote:Would you suggest any improvements?...


  • Take out all the code from Main method
  • Double.parseDouble is called two times on array[0] and array[1] respectively. It can be called only once instead
  • "derive" should be changed to "divide"
  • If you want to write good Character User Interface (CUI) programs, do not expect user to type in full words as commands. Specially if you support only 4 commands
  • Have you taken care of division by 0 ? Your program would print "Infinity" in this case.
  • How is the user aware of your "stop" feature without reading this code ?
  • The scope of "array" exceeds it's use
  • Use meaningful variable names
  •  
    Mike Savvy
    Ranch Foreman
    Posts: 74
    2
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hello guys, thank you very much for the reviewing!
    I have changed the program so far, even though I use only one method because I do not see reason to do more (maybe only for better readability). What would you say?
     
    Campbell Ritchie
    Marshal
    Posts: 68044
    258
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Mike Savvy wrote:. . . I use only one method . . .

    Nonononononononono. Multiple methods are not there for readability but for reliability. We have already told you not to use the main method alone.

    You might say thank you, but I think you have ignored most of the advice given, and I think that code is worse than its original version.
     
    Marshal
    Posts: 25183
    64
    Eclipse IDE Firefox Browser MySQL Database
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Sometimes you use just "continue" and other times you use "continue CALCULATOR". It looks to me like they both do the same thing, in which case you should use one or the other. Using both of them leads to confusion since the reader has to wonder why you used different forms which do the same thing.

    However I can't tell for sure whether they do the same thing because your method is way too long and it's too hard to follow the indentation down the page. Your dismissive comment "only for readability" is just wrong -- if people can't read your code then its value is diminished. And when I say "people" I include you, since as a real-life programmer you would often have to come back to look at code which you wrote a week, or a month, or a year ago. Don't rely on remembering what the code was supposed to do, it should tell you.

    So yeah, you need to break that loop up. It's doing about three different things so a few methods would improve it greatly.
     
    Marshal
    Posts: 6824
    182
    Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Here's a link to a quick article about HowToFormatCode (that's a link).  If you format as you code it will make things a lot easier.
     
    salvin francis
    Saloon Keeper
    Posts: 2511
    117
    Google Web Toolkit Eclipse IDE Java
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    It's been a couple of days, but I thought I'd still follow up on this thread. Just wanted to point out, your new code is much worse than the original code. Look carefully at all our instructions above.
     
    If you have a bad day in October, have a slice of banana cream pie. And this tiny ad:
    Java file APIs (DOC, XLS, PDF, and many more)
    https://products.aspose.com/total/java
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!