trjhtr

I'm looking to get some advice on whether or not I am building/structuring my application the correct way. The program has a few key components:

• Calculate the fine based on how fast over the limit a driver was travelling


• Generate a fine letter address to the driver


• Update a file with a driver's registration details and the fine

So that being said, I have the following classes:

• DriverDetails


• DriverManager


• Fine


• FineCalculator


• FineType

Here is the idea:

When I want to work out the fine:

1. 
   FineCalculator object is created


2. The speed limit is set


3. The speed of the car is set and then the calculateFine() method is called


4. The calculateFine() method will return a Fine object, which is a data object which contains an amount, and a string (the result), the points if any, a FineType
   

This is one side of the application.

The other part is to be done after a fine has been generated. The user will enter the driver details, a DriverDetails object will be created, for the user to then generate any form of documentation it will use the DriverManager object which will take a DriverDetails object when constructed.

The FineCalculator class:

public class FineCalculator 

 private int speedlimit;

 private int carspeed;


 public FineCalculator(int speedlimit, int carspeed) 

 this.speedlimit = speedlimit;
 this.carspeed = carspeed;
 


 public Fine calculateFine() 

 Fine fine = new Fine();

 int milesover = carspeed - speedlimit;

 if(milesover < 1) 

 fine.result = "The user was not going above the speed limit, no actions will be taken.";
 

 else if(milesover < 5) 

 fine.result = "No fine issued, no points gained but a warning will be issued to the driver.";
 fine.finetype = FineType.WARNING;
 

 else if(milesover > 4 && milesover < 10) 

 fine.result = "A fine of £50.00 will be issued to the driver. 0 points gained.";
 fine.amount = 50.00;
 fine.finetype = FineType.FIFTY;


 

 else if(milesover > 9 && milesover < 15) 

 fine.result = "A fine of £100.00 will be issued to the driver. 0 points gained.";
 fine.amount = 100.00;
 fine.finetype = FineType.ONE_HUNDRED;

 

 else if(milesover > 14 && milesover < 20) 

 fine.result = "A fine of £150.00 will be issued to the driver. 3 points gained.";
 fine.amount = 150.00;
 fine.points = 3;
 fine.finetype = FineType.THREE_POINTS_ONE_FITY;
 

 else if(milesover > 19) 

 fine.result = "A fine of 1000.00 will be issued to the driver, and they will be disqualified.";
 fine.finetype = FineType.DISQUALIFIED;
 

 return fine;

 

The Fine class:

public class Fine 

 public Double amount;
 public String result;
 public int points;
 public FineType finetype;

The DriverDetails class:

public class DriverDetails 

 public String firstname, surname, address, city, postcode;
 public int drivernumber;

The DriverManager class:

public class DriverManager 

 private DriverDetails driverdetails;
 private Fine fine;

 public DriverManager(DriverDetails driverdetails, Fine fine) 

 this.driverdetails = driverdetails;
 this.fine = fine;
 

 public void generateFineLetter() 


 

 public void updateDriverDetailsWithFine() 


 

And finally, the simple enum class:

public enum FineType 

 WARNING, FIFTY, ONE_HUNDRED, THREE_POINTS_ONE_FITY, DISQUALIFIED

My biggest concern is the fact that I'm not using the getters/setters inside the DriverDetails and Fine data classes. I mean, it doesn't feel like the getters and setters are completely necessary. Am I wrong? I know what I'm doing goes completely against encapsulation, but in this scenario is that bad? Can I have any advice here as to what to improve or if I'm structuring this wrong?

It is a question of how much will the project expand beyond what is described here. For instance, are you going to persist driver details and other data? if you are going to use a database, then you will probably want to use an Object-Relation-Mapping (ORM) framework, and they tend to expect Java Beans to work with. perhaps you will want to develop a web api that will send or receive such data in json format? json parsers/generators also prefer objects that follow Java Bean standard.

for very simple, small, one-man projects it is probably ok to skip the boilerplate code of getters/setters. it is just that usually projects tend to grow and expand in time.

another point that can and should be addressed now is the calculator: it feels wrong to me to create instances just to do fine calculation. if we make the analogy to windows calculator, it is like openning a new window each time you want to do a new calculation. the calculator is stateless: it takes arguments and produces a result. it does not carry any state between calculations. So I would make the calculateFine() method static and have it accept the two arguments.

another issue is the calculation process itself. all the attributes of a fine are closely related to each other and come in mutually exclusive sets: the result is a textual description of the enum, the amount and points are also linked to it.

I would make these values properties of the enum:

public enum FineType
 
 NONE("The user was not going above the speed limit, no actions will be taken.", 0, 0), 
 WARNING("No fine issued, no points gained but a warning will be issued to the driver.", 0, 0), 
 FIFTY("A fine of £50.00 will be issued to the driver. 0 points gained.", 50.00, 0), 
 // and so on

 private String description;
 private int fine;
 private int points;

 FineType(String description, int fine, int points) 
 this.description = description;
 this.fine = fine;
 this.points = points;
 

 public String getDescription() return description; 
 public String getFime() return fine; 
 public String getPoints() return p[oints; 

now you can have the Fine take a type in the constructor and fill the properties by itself. this makes the calculator process clearer:

public static Fine calculateFine(int speedlimit, int carspeed) {

 int milesover = carspeed - speedlimit;

 if(milesover < 1) 
 return new Fine(FineType.NONE);
 
 else if(milesover < 5) 
 return new Fine(FineType.WARNING);
 
 ...

The problem with your design is that you have different fields that, to a certain extent, represent the same property, leading to interdependent state, which makes the code fragile and confusing. I am talking about the class Fine and FineType. For example, the class Fine contains a field amount, but it also contains a field finetype, both of which represent information about the size of the fine. So it is possible to construct an invalid Fine object by assigning the fields amount and finetype contradictory values, e.g. amount = 50 and finetype = FineType.ONE_HUNDRED. The same applies to the number of points gained.

Since the fields amount and points already contain all the necessary information regarding the the fine size and the number of points, the question is why you actually need an enum. 3 of the 5 enum values, namely FIFTY, ONE_HUNDRED and THREE_POINTS_ONE_FITY, only represent information that is already stored in Fine.amount and Fine.points, which means that these enum values are practically useless. So the question arises what the enum is actually supposed to represent. Let's look at the other two values, WARNING and DISQUALIFIED. Disqualification is an additional action that is taken on top of the fine, and a warning is issued in place of a fine, so both are not really "types" of a fine, but actually have nothing to do with a fine directly.

So how to represent a warning? That probably depends on what you want to do with a warning. Does it have any state associated with it, like a Fine object that contains the size of the fine and the number of points gained? If so, it could be a class of its own (and File and Warning could be subclasses of a common superclass). But judging by your code sample, it doesn't look like it, so maybe it would suffice if the file that contains the driver's registration details has a way of storing the warning without the warning itself being represented as an object, for instance a List of dates on which a warning has been issued to the driver.

Similarly, the disqualification is not directly related to the fine, so like the warnings, it could simply be stored in the file with the driver's registration details separately from the fines.

Finally, about the field result in the class Fine. This should not be a field at all, because its content only depends on the other fields. Instead, I would write a method that generates the String based on the values amount and points, for example like this:

public String getMessage() 
 return String.format("A fine of £%1$.2f will be issued to the driver. %2$d points gained.", amount, points);

Depending on what you want to do with the class, you could also consider overriding toString() to generate this String.