Getter method usage

Question

I have a scenario where a seller have following fees configured:

BigDecimal paidPlanFixedFee
BigDecimal fixedFee
Boolean isPaidPlan

Now, if seller has paid plan it should use paidPlanFixedFee while triggering payments and fixedFee if seller is on free plan.

One approach is to change the code everywhere where fixedFee is referred like:

if(paidPlan){
// Use paidPlanFixedFee
}else{
// Use fixedFee
}

Other approach is to change only in the getter method of fixedFee :

BigDecimal getFixedFee(){
  if(paidPlan){
    // return paidPlanFixedFee
  } else{
   // return fixedFee
  }
}

Is it a good practice to use the getter method approach in this scenario or should it be avoided ?

This kind of questions fit https://codereview.stackexchange.com/ better. — Szymon Stepniak, Nov 02 '18 at 07:43
Personally, I see nothing bad in `getter` like that but I'd move this logic a bit higher and apply extension. — Opal, Nov 02 '18 at 07:43
In case you go with the `getter` approach i would suggest to change the name of your getter as now it will look like it always returns the `fixedFee` whilst in reality it can also return the `paidPlanFixedFee`. — Bas de Groot, Nov 02 '18 at 07:47
https://stackoverflow.com/questions/7955319/getters-and-setters-performing-additional-logic — Prasanth Nair, Nov 02 '18 at 07:53
@JoeC is right. Why not simply fee? It seems that you have a single entity that can have either free or paid access to a resource. You can that have an abstract entity with 2 subclasses or a single field called fee and move the if elsewhere. — Opal, Nov 02 '18 at 08:17
@Opal I was also thinking of using a third transient field named "currentFee" that would simply return either fixed or paid fee. All the reference to fixedFee should now refer to currentFee. — Vivek Sadh, Nov 02 '18 at 08:21

score 1 · Answer 1 · edited Nov 26 '18 at 06:44

Looks like overriding getter with this logic is not a good idea. In this case you should control every getter usage (for example json serialization, or db model mappings etc.) because It may cause troubles if somewhere you'll need to get original value.

Better to have separate calculateFee() method

And what about transient field?

If you'll create getter without actual field in groovy:

BigDecimal getFee(){
  paidPlan ? paidPlanFixedFee : fixedFee
}

then you can access it like a regular field:

println instance.fee

In this case you'll need to change fixedFee to fee where fixedFee is referred. But after this changes all future modifications will be easier.

Getter method usage

1 Answers1