• 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

Meter Reader

 
Greenhorn
Posts: 22
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hey just put this together.

Any comments on how I can improve this code? It achieves what I want it to achieve but keen to learn best practices.







Adam
 
Bartender
Posts: 1251
87
Hibernate jQuery Spring MySQL Database Tomcat Server Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Adam Turner wrote:It achieves what I want it to achieve but keen to learn best practices.

If I'm not wrong, do you want this program to just print last reading, current reading and their difference ? and probably modelNumber also?
If not only these Or something more then first you need to jot down all those things you want this program to do for you.
 
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Adam Turner wrote:It achieves what I want it to achieve but keen to learn best practices.

Have a cow for a good mindset.

Some comments will provide later today.
 
Sheriff
Posts: 7125
184
Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Since you have instance variables in MeterReader, why not use them?  Then, take both arguments out of displayInfo() and calcDifference().

The method calcDifference() does two things: calculate the difference in meter readings and display this difference.  A method should only do one thing.  I would move the println to the displayInfo() method.

Next, do you want to call calcDifference() in displayInfo(), or better yet, using the setters for lastReading and currentReading and always calculate the difference when setting either one.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Likes 1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Adam,

Here are some comments:

FITCalcv1 class
-------------------
1. Class name, FITCalcv1? v1 I pressume is version number, how about when you'll have v2? Will you copy/paste all that code to other source file and will name v2? Don't. Exclude such things from class names. You could have it in comments form if you want it. Lets assume you ended up with FITCalc. What does it tell you? To me not much honestly. Calc I could guess what it means - calculator. What about FIT? Looking to program code, can't decypher. Try to think about the better name.

2. Variable names. reading1. Why 1? user_input, it doesn't follow Java variables naming convention. That should read as userInput. Would suggest to improve it. Next - lr, cr. Try to remove everything what is after equals sign, what this variable itself lr or cr tells you? Nothing. Variable name should tell you exactly what it represents right away by looking to it, without any effort looking further into the code. I'd name them - lastReading, currentCreading. In Fact, you have them already, but for some reason you decided to used them for a very short period of time. Are you aware that you can read integers, decimal numbers by using methods nextInt(); nextDouble()?

3. Why you have some spaces before and after parentheses?
why you don't have them here then?
and here
here
why those spaces appeared here again? In fact after 'd' there isn't one
here again

Can you see the inconsistency in your formatting? If you think it isn't important, you're mistaken. If you get chance, read the book "Clean Code" by Robert C. Marting. It might will change the way you think about code consistency, about which many people think it isn't important. In fact, it is important, and equally than any other issues in code. I'd avoid spaces before and after parentheses when we talk about paremeters.

4. Can you notice how you duplicate your code within FITCalcv1 class? Lines 10 - 13 and lines 15 - 18. Consider having method which accepts prompt message and returns user input. Might move such method to a Utility class. Look in this forum for Utility class, you should find some examples.

MeterReader class
--------------------
1. Shouldn't MeterReader have a constructor so you could pass lastReading, currentReading and modelNumber to it and create MeterReader object? So later by using internal data you could use it to calculate difference, display information and any other related information.


Well, enough probably for now, try to improve your code and lets see what others will advice you.
 
Liutauras Vilda
Marshal
Posts: 8856
637
Mac OS X VI Editor BSD Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Adam, have you learned about the interfaces? There is a method or two which in my opinion are well suited to provide in an interface type, so later you could provide concrete implementation in the classes of interest. In reality you could and you do have many different types of meter readers (GasMeterReader, ElectricMeterReader, WaterMeterReader), so having common interface they would all implement could be handy.

Also, are you aware of toString() method?
 
Ganesh Patekar
Bartender
Posts: 1251
87
Hibernate jQuery Spring MySQL Database Tomcat Server Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I think rather than taking input inform of String and parsing It to double, you can declare lastReading and currentReading as double and use nextDouble() method of Scanner for input.
 
Adam Turner
Greenhorn
Posts: 22
1
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hey sorry for no reply. I'll give a good response later today if I can. Very busy at the moment.
 
reply
    Bookmark Topic Watch Topic
  • New Topic