Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

After coding for many years as a solo programmer, I have come to feel that most of the time there are many benefits to write private member functions with all of the used member variables included in the parameter list, especially development stage.

This allow me to check at one look what member variables are used and also allow me to supply other values for tests and debugging. Also, a change in code by removing a particular member variable can break many functions. In this case however, the private function remains isolated am I can still call it using other values without fixing the function.

Is this a bad idea afterall, especially in a team environment? Is it like redundant or confusing, or are there better ways?

share|improve this question
    
Call my confused, but you can access the class fields within the class without passing them as parameters. What's the use case? –  Oded Nov 29 '11 at 17:53
    
I think I explained it in my second paragraph? –  Jake Nov 29 '11 at 17:55
    
outside of your convenience, what use case? –  Oded Nov 29 '11 at 17:56
    
convenience is the whole point, during development stage. I know member variables can be accessed directly, otherwise I wouldn't have asked this question. Your comments hint that you wouldn't do this even out of convenience, so could you share why so? Thanks. –  Jake Nov 29 '11 at 18:01
3  
Do you go back and refactor your code so that it uses class members instead of parameters once the 'development stage' is done? Or do you leave things the way they are once you get it working? In other words, do you allow your desire for short-term convenience to impact the final class interface? –  William Shakespeare Nov 29 '11 at 18:08

5 Answers 5

up vote 8 down vote accepted

allow me to check at one look what member variables are used

Using a method signature solely to identify members of a class is a crude, non-OO style of programming. OOP is about higher level abstractions and relationships. If you are having trouble tracking a ridiculous # of members, then you should immediately review your classes to ensure they have a single and easily identifiable purpose. This sounds like you are constructing god objects.

Also, a change in code by removing a particular member variable can break **many** functions.

A great indicator of tight coupling. Decouple your classes. Look at dependency injection..

allow me to supply other values for tests and debugging.

Unclear on meaning here. Especially because you go on to say

In this case however, the private function remains isolated am I can still call it using other values without fixing the function.

Generally, private functions make code more difficult to (unit) test not easier. Are you familiar with Mocking? Are you testing with unit tests, some kind of home brew or other?

share|improve this answer

Is this a bad idea afterall, especially in a team environment?

I'd say it's at least a questionable practice that could lead to trouble, particularly in a team environment. Having worked this way for some time, you may have developed habits and understanding which avoid the potential trouble, but team members are likely not to understand your unwritten rules.

In particular, it seems like you're essentially practicing procedural programming in an object-oriented environment. The point of objects is that they contain both the data and the operations on that data. If you tell a view to draw(), for example, you don't need to tell it what to draw -- it should draw itself. If you have to tell it what to draw, you're not taking advantage of the power of OOP.

A danger of your approach is that your methods are operating not on the object's members, but on the parameters. If you pass those parameters by value, any changes to them won't be reflected in the corresponding member variables. You can solve that problem by passing by reference, but in that case your code might change member variables if that's what you pass in, but it's not obvious that that's what's happening. Worse, there's the very real possibility that the caller will pass in something other than a member variable, which really changes the meaning of what the method does.

Is it like redundant or confusing

  • Redundant: definitely.
  • Confusing: apparently not to you, but surely to others.

are there better ways?

Use member variables or properties instead of parameters in methods that are intended to operate on data stored within the object. Use parameters for information that's external to the object.

If you have trouble discerning which members a given method uses, try any of:

  • Put a comment near the beginning of the method that explains what the method does, lists the affected members, and perhaps describes any output.

  • Simplify your methods. I'm all for appropriate comments, but code is much nicer when you can scan the code quickly and get a pretty good idea of what's going on.

  • Stop worrying about it. A method should use whatever internal data it needs to do its job. Code outside the method should be more concerned with what the method does, not how it does it.

share|improve this answer

If you want to have the function in class (private or not, does not matter), use members as members, so that it is an operation of the class. Otherwise it does not, in principle, belong in the class. Put it out of the class then and call it with lots of params. If you feel it is a helper that "must work no matter how the class is refactored".

share|improve this answer

In general, it's not a good idea to give methods (or anything for that matter) access to information it doesn't need. Having a large list of parameters means sifting through more of them when you or someone else needs to find something. If they are definitely related and necessary, maybe you should consider creating an object to pass, or passing the object from which they came.

As far as testing goes, you should try to keep tests separate from your actual logic.

share|improve this answer

As others have pointed out what you describe seems to go against the idea of objects, for a thorough explanation of this I'd recommend Robert C. Martins book "Clean Code" especially the chapter on methods.

It's relevant because it is a thorough and practical description of how to write clean object oriented code. The referenced chapter in particular discusses method parameter lists regarding:

  • their length,
  • the method's responsibility,
  • the class' responsibility,
  • and the instance variables.

That chapter contains a long answer to the question. The short answer is to have few parameters, to favor instance variables, and to keep classes very focused - that is in accordance to the Single Responsibility Principle.

share|improve this answer
    
Christian I edited the above comment in the answer. In the future, when asked for clarifications or expansion of your posts, please update the post instead of responding in comments. Thanks for taking the time to improve your answer. Oh, and please remove your comment, as it's now obsolete. –  Yannis Rizos Jan 15 '12 at 7:41

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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