Note: there is a similar question that addresses my subject: Better style for member variables?
... but that question does not address "dealing with large legacy code base and comprehension of functions that grew too large". I am looking for confirmation or rejection of whether my situation warrants it to be a is a good enough reason to adopt a practice of writing internal-to-the-class functions as if they were separate entities, with their appropriate parameters and return values.
My question
Variables internal to a class are considered to be scoped as "globals" within contains of that class. As such, these variables do not need to be included explicitly as parameters to the class' internal functions. Further, if internal functions return values that may belong to existing inside-class variables, those also do not need to be returned explicitly. You can just refer to them via using this object. But, is doing so a good practice?
My question comes from the context of refactoring a huge legacy codebase. As such, when I see a function inside a class that goes like $this->IamACute3000LineFunctionDoingInsanelyCuteStuffInASpaghettiCodeLikeWay(), I have little clue as to which in-class "global" variables it needs to do its job, and what exactly it returns. This makes it hard and time consuming to deal with that code when any work is to be done on the class. Thus, even though it may not make sense inside the class itself, I do see some merit in writing code like below, treating variables and parameters as if the function was written to be used outside of the class itself:
class HelloClass
{
$private $name;
$private $greeting;
public function __construct($name)
{
$this->name = $name;
//line of interest
$this->greeting = $this->prepGreeting($this-name);
}
//function of interest
private function prepGreeting($name)
{
return "Hello, $name";
}
public function getGreeting()
{
return $this->greeting;
}
}
$sayHello = new HelloClass("Dennis");
print $sayHello->getGreeting();
Here, prepGreeting is the function in question. Surely, it looks a bit ridiculous... as it barely takes advantage of built-in class mechanisms, if at all. It can be rewritten as such instead (with appropriate other constructor changes as well, which are not shown):
private function prepGreeting()
{
$this->greeting = "Hello, " . $this->name;
}
But then you have to actually read the function's body to understand what it does, what it touches and what it returns.
My question is this:
in my example, where I am dealing with large code where understanding a function takes time, I think there is a good reason to write internal class functions as if they were separate, not belonging to the class, as it will be superior for understanding what goes on inside the function, and what it returns. This function will be self-contained and as such, it will help a lot with refactoring. I could pluck that function out, bend the way I need it to bend, move it to another class, if I have to, thus making it easier to work with the class itself. But, in general, would you recommend such function-writing practice when writing/designing classes?
Or would you recommend instead to use built-in this object, treating internal to the class variables as "class globals", even though doing so may impact readability/understanding/comprehension of the function if its size does grow, as it often does in real life projects?
Finally, am I missing any greater points while focusing on this specific function-writing-style issue?
Add-on
I do see merit in perhaps using some middle-ground, where function parameters are not passed to the function (function uses this object to get to those), but the function still returns values, instead of updating this object directly, and it is up to the caller to update this variable. i.e. this will change my "line of interest" to this code: $this->greeting = $this->prepGreeting(); and at least I will know what the function is supposed to return, at a glance.
