• Post Reply Bookmark Topic Watch Topic
  • New Topic

Meter Reader  RSS feed

 
Adam Turner
Greenhorn
Posts: 22
1
  • Mark post as helpful
  • send pies
  • 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
 
Ganesh Patekar
Bartender
Posts: 696
23
Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
  • Mark post as helpful
  • send pies
  • 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.
 
Liutauras Vilda
Marshal
Posts: 4666
320
BSD
  • Mark post as helpful
  • send pies
  • 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.
 
Knute Snortum
Sheriff
Posts: 4091
112
Chrome Eclipse IDE Java Postgres Database VI Editor
  • Likes 1
  • Mark post as helpful
  • send pies
  • 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: 4666
320
BSD
  • Likes 1
  • Mark post as helpful
  • send pies
  • 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: 4666
320
BSD
  • Mark post as helpful
  • send pies
  • 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: 696
23
Eclipse IDE Hibernate Java jQuery MySQL Database Netbeans IDE Oracle Spring Tomcat Server
  • Mark post as helpful
  • send pies
  • 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
  • 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.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!