views:

66

answers:

3

Example:

// access fields directly
private void doThis()
{
    return doSomeWork(this.data);
}

// receive data as an argument
private void doThis(data)
{
    return doSomeWork(data);
}

The first option is coupled to the value in this.data while the second option avoids this coupling. I feel like the second option is always better. It promotes loose coupling WITHIN the class. Accessing global class data willy-nilly throughout just seems like a bad idea. Obviously this class data needs to be accessed directly at some point. However, if accesses, to this global class data can be eliminated by parameter passing, it seems that this is always preferable.

The second example has the advantage of working with any data of the proper type, whereas the first is bound to working with the just class data. Even if you don't NEED the additional flexibility, it seems nice to leave it as an option.

I just don't see any advantage in accessing member data directly from private methods as in the first example. Whats the best practice here? I've referenced code complete, but was not able to find anything on this particular issue.

+2  A: 

In class global data are not a problem IMHO. Classes are used to couple state, behaviour and identity. So such a coupling is not a problem. The argument suggests, that you can call that method with data from other objects, even of other classes and I think that should be more considered than coupling inside class.

Gabriel Ščerbák
+1  A: 

They are both instance methods, therefore #1 makes more sense unless you have a situation involving threads (but depending on the language and scenario, even then you can simply lock/mark the data method as syncronized - my Java knowledge is rusty).

The second technique is more reminiscent of procedural programming.

Chris S
+3  A: 
  • if the data is part of the object's state, private/protected is just fine. option 1 - good.

  • i noticed some developers like to create private/protected vars just to pass parameters between methods in a class so that they dun have to pass them in the method call. they are not really to store the model/state of an object. ...then, option 1 - NOT good.

Why option 1 not good in this case...

  • expose only as much as you need (var scoping). so, pass the data in. do not create a private/protected var just to pass data between 2 methods.
  • private methods that figures out everything internally makes it very easy to understand. keep it this way, unless its unavoidable.
  • private/protected vars make it harder to refactor as your method is not 'self encompassing', it depends on external vars that might be used elsewhere.

my 2 cents! :-)

koss