2

This question already has an answer here:

I believe this is language agnostic question - if it's not then please correct me.

Let's say I have a class (code snippet is a kind of 'pseudo code')

class Car:
  private steering_wheel
  private engine
  public Car(steering_wheel, engine):
    this.steering_wheel = steering_wheel
    this.engine = engine


  public void service():
     clean_steering_wheel()
     repair_engine()

  private void clean_steering_wheel():
    this.steering_wheel.do_stuff()

  private void repair_engine():
    this.engine.do_stuff()

which works well! But I could also change private methods a bit and let them use fields (i.e steering_wheel and engine) via arguments, not directly

   class Car:
  private steering_wheel
  private engine
  public Car(steering_wheel, engine):
    this.steering_wheel = steering_wheel
    this.engine = engine


  public void service():
     clean_steering_wheel(this.steering_wheel)
     repair_engine(this.engine)

  private void clean_steering_wheel(steering_wheel):
    steering_wheel.do_stuff()

  private void repair_engine(engine):
    engine.do_stuff()

Let's assume that all parameters(arguments) are passed by reference.

What are the advantages and disadvantages of these solutions? Which is considered as better practice? Which one do you prefer?

marked as duplicate by user40980, user22815, Scant Roger Nov 19 '15 at 6:10

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

2

Sounds very much a judgement call to your specific situation, but a couple considerations come to mind. If the methods are private, and you don't have any reason to pass the parameter, I would probably avoid it if for no reason other than it being less complex. You could also argue that there is a performance reason to avoid it, as there is no reason to pass additional parameters on the stack if they aren't needed.

One time I do find that I do pass a member as a parameter is with protected members. Let's say a child class does something like add a second steering wheel that also needs to be cleaned. In that case, it could (presumably) re-use the clean_steering_wheel method from the base class and pass a different steering wheel member.

  • Off topic questions happen a lot here. Trying to answer them only encourages the behavior. Recommended reading: Should one advise on off-topic questions?. – Adam Zuckerman Nov 17 '15 at 2:00
1

Passing the field as an explicit parameter is better if you're not invoking a method, but a function/static method: The less data is available in a function, the more explicit all dependencies are, the easier it is to reason about that code. So a function clean_steering_wheel(steering_wheel) will not surprise a user by actually doing something with the engine.

However, a (private) method will already receive references to the engine and steering_wheel via the implicit this parameter. If you have a this parameter in addition to an explicit steering_wheel parameter, you are duplicating your data – which can lead to inconsistencies. In particular, I'd be wondering why you didn't use the steering_wheel from this: Are you operating on the steering_wheel of a different Car? On a steering_wheel that does not belong to any Car?

In light of this, it would be less confusing to define clean_steering_wheel as

private void clean_steering_wheel():
  this.steering_wheel.do_stuff()

However, it would be even better to use a free function:

void function clean_steering_wheel(steering_wheel):
  steering_wheel.do_stuff()

void function repair_engine(engine):
  engine.do_stuff()

class Car:
  ...

  public void service():
    clean_steering_wheel(this.steering_wheel)
    repair_engine(this.engine)

, or static method:

class Car:
  ...

  public void service():
    clean_steering_wheel(this.steering_wheel)
    repair_engine(this.engine)

  private static void clean_steering_wheel(steering_wheel):
    steering_wheel.do_stuff()

  private static void repair_engine(engine):
    engine.do_stuff()

Which precise approach to use is quite language-specific, and also has impact on testability. E.g. in C++, using free functions in an anonymous namespace affords the best encapsulation, but actively hinders testability. Private static member functions might be better, since they can be tested by a friend class. In Java, you can't have free functions but must use “static methods”, so the discussion is unnecessary.

Not the answer you're looking for? Browse other questions tagged or ask your own question.