tags:

views:

65

answers:

3

Hi,

I need to write some instance method, something like this (code in ruby):

def foo_bar(param)
  foo(param)
  if some_condition
    do_bar(param)
  else
    do_baz(param)
  end
end

Method foo_bar is a public api. But I think, param variable here appears too many times. Maybe it would be better to create an private instance variable and use it in foo, do_bar and do_baz method? Like here: (@param is an instance variable in ruby, it can be initialized any time)

def foo_bar(param)
  @param = param
  foo
  if some_condition
    do_bar
  else
    do_baz
  end
end

Which code is better? And why?

A: 

If you do not need param outside of the foo_bar method the first version is better. It is more obvious what information is being passed around and you are keeping it more thread friendly.

And I also agree with Mladen in the comment above: don't add something to the object state that doesn't belong there.

ormuriauga
personally, I would remove all the parentheses though.
ormuriauga
+1  A: 

The first version should be preferred for a couple of reasons. First, it makes testing much easier as each method is independent of other state. To test the do_bar method, simply create an instance of its containing class and invoke the method with various parameters. If you chose the second version of code, you'd have to make sure that the object had all the proper instance variables set before invoking the method. This tightly couples the test code with the object and results in broken test cases or, even worse, testcases that should no longer pass, but still do since they haven't been updated to match how the object now works.

The second reason to prefer the first version of code is that it is a more functional style and facilitates easier reuse. Say that another module or lambda function implements do_bar better than the current one. It won't have been coded to assume some parent class with a certain named instance variable. To be reusable, it will have expected any variables to be passed in as parameters.

The functional approach is the much better approach ... even in object oriented languages.

Hitesh
+1  A: 

Is param replacing part of the state of the object?


If param is not changing the object state then it would be wrong to introduce non-obvious coupling between these methods as a convenience.

If param is altering the state of the object then it may still be bad practice to have a public api altering the state - much better to have a single private method responsible for checking and changing the state.

If param is directly setting the state of the object then I would change the instance variable here but only after checking that the new state is not inconsistent

Chris McCauley