2

PROBLEM: this points to different instances at different stages of execution (as it should). But it is not always obvious which instance is that. On the other hand we could minimize the use of this, fixing its state with meaningful-name variables.

EXAMPLE:

// Variant A:
this.someFunction();

// Variant B:
var combobox = this;
combobox.someFunction();

QUESTION: Is it a good or a bad style to replace this pointer with variables and why?

12
  • 3
    Improving code clarity is the reason we have "good style." Which style do you believe provides better clarity? Commented Jan 14, 2016 at 5:28
  • 1
    Not much skilled, so want to check some points. I find the second clearer. Currently refactoring a code with lots of this and want to replace it. Maybe there are general dangerous cases where this shouldn't be replaced?
    – Zon
    Commented Jan 14, 2016 at 5:34
  • 2
    I think that you'll find the answers to be based heavily on what language you are using specifically. Maybe it would be better if your question focused on the specific language you are currently working in.
    – Caleb
    Commented Jan 14, 2016 at 5:40
  • 4
    "it is not always obvious which instance is that" - at least in the languages I know, it is pretty obvious from the method context.
    – Doc Brown
    Commented Jan 14, 2016 at 5:49
  • 4
    @Zon The question is not language agnostic at all. The behavior you described is something quite specific to and odd about JS as far as I know. Think about C++/Java/C# - this is always pointing to an object of the class containing the current method. It could be a derived class, but that's it basically. Commented Jan 14, 2016 at 12:12

2 Answers 2

8

I think there are two layers to this question (pun intended)

Note: I assume that you are referring to javascript, and I base my answer in that language.

1) this refers to the object that invokes the function
(or the object that the function was bound too, but we don't need to get into all of that...)

Why is this important?
Because, in javascript, functions are objects and can be set as properties on other objects. If you are not careful with your function scope, it is easy for things to go wrong (especially when wiring up events/callbacks).

If you use a variable that locks this to the instance of the object, you are creating a "closure" in the function.

example (note what is being alerted):

var o1 = new function (){
    this.name = "o1";

    this.m1 = function () {
        alert(this.name);
    };
};

o1.m1(); // alerts "o1"

var o2 = new function () {
    this.name = "o2";

    this.overrideM1 = function () {
        alert(this.name + " override");
    };
};

o1.m1 = o2.overrideM1; // replace the function
o1.m1(); // alerts "o1 override"

var o3 = new function () {
    var self = this;

    self.name = "o3";

    self.overrideM1 = function () {
        alert(self.name + " override2"); // note the use of "self" instead of "this".
    };
};

o1.m1 = o3.overrideM1; // replace the function
o1.m1(); // alerts "o3 override2", because we have a "closure" in the function to the "o3" instance.

So, how does this relate to your question?

Variant A and Variant B are not necessarily as "equivalent" as you might think when you look at all the code in your project.

Some places might need to use the this literal inside a function, because it gets used on different objects and needs the correct scope when called on those objects. Other places might assign this to a variable to create a closure in a function for a specific reason.

It's not a matter of which one is "better" because they are functionally different.

2) But what about in the places where we don't have to worry about scope, and the two variants are actually functionally the same?

In these cases it would be best to come up with a convention that your team agrees with and will use consistently. Either would work.

In my opinion, Variant A is the cleaner of the two (assuming that your code is designed well) If it is not obvious what this is referring to in the code you are looking at, it's probably a sign that you need to clean up your code. Adding more variables all over the place is just going to make more clutter in an already "confusing" code base.

2
  • The question really came from a closure situation: functions encapsulation and dynamic scope.
    – Zon
    Commented Jan 14, 2016 at 8:51
  • 1
    Which is kind of the point I was trying to make in my example. Once you are dealing with closures, using this vs using a "variable" can give you completely different scopes when the function gets executed. So the question becomes less about what looks better, and more about what gives you the correct scope. The "correct" thing to do is based entirely on the situation.
    – Caleb
    Commented Jan 14, 2016 at 8:54
1

QUESTION: Is it a good or a bad style to replace this pointer with variables and why?

To me, if we try to go as language agnostic as possible, then a lot of the temptation to give this a more meaningful name seems kind of counter-productive.

Focus on Abstract Interfaces

First, a lot the flexibility of languages that allow you to write this.someFunction() where this could point to different data types at different times within the same function can benefit from that flexibility. In such a generic context, it shouldn't matter to the correctness of the function what this is:

function impale(stick)
{
    // Kill this thing. It doesn't matter what it is in the context
    // of this function, all that matters is that it can be killed.
    this.kill();
    stick.combine(this);
}

In such a case, the impale function shouldn't care about what is actually implicitly passed through it (what object it is invoked on). All it should care about is that it can be killed, made dead, and can be combined with a stick (though some of that passes responsibility passes over to stick). If someone passes something which cannot be killed, that's an error in the one calling the function, and not the callee (the function) itself.

In such generic contexts, to try to assign a variable name as a sort of alias defeats a lot of the purpose:

function impale(stick)
{
    // This kind of documentation suggests this function only works on cats.
    var cat = this;
    cat.kill();
    stick.combine(cat);
}

... same goes for this (simply a comment):

function impale(stick)
{
    // Kill a cat.
    this.kill();
    stick.combine(this);
}

To add this kind of type-based information, even in the form of a comment, might actually place an unhealthy focus on concrete details over abstract interfaces: "what is this exactly?" vs. "what can this do regardless of what it actually is?"

Focusing much more on the latter, and caring less about the former, will generally enhance your ability to write code that works in a wider range of scenarios (and therefore ultimately write less code to maintain). Yet, of course, it is hard to do this perfectly. In debugging scenarios, for example, what something actually is might start to become very relevant, yet that's something most decent debuggers will show in a watch window when examining what this is.

Superfluous, Mutable State

Taking the above example though, I'd say the comment route is actually preferable to aliasing this through a mutable variable, in cases where there is no logical difference and the local variable only serves as an alias for documentation purposes.

It's because the complexity of a function to human interpreters is often multiplied by the amount of mutable state that is accessible. This is especially true in a team-wide setting where we don't even know what the other members of the team did exactly to every single line of code in every function until we carefully decipher them line-by-line. A function that has 300 lines of code and 20 mutable variables at the top and access to 5 global variables at module/file/class scope, e.g., will feel like it has an immediate state complexity of 25*300 = 7500. Meanwhile a function that is 20 lines of code with access to one local variable in the middle of the function (10 lines of code capable of accessing it) and no global variables will feel more like it has a state complexity of 1*10 = 10.

As asinine as it might seem to think that the introduction of a cat variable above might lead to a higher potential of error ("no one would ever mess with that and set it to anything else"), in a wide team setting, you start to see every kind of goofy mistake imaginable at some point, accompanied by a strong appreciation for Murphy's law.

This idea of state complexity is kind of a rough metric I made up (perhaps best formally expressed in terms of coupling), and we don't perfectly think this way, but the more mutable state is exposed to each line of code, the more it tends to feel complex, the more room there will seem to exist for error, since the more our mental stack has to account for potential changes made to all such accessible states.

This is one of the reasons why it's generally preferable to give variables a narrower scope (shorter functions, variables declared more locally within shorter scopes, classes with fewer member functions that have access to internal states, smaller files if they have access to file-scope global variables, etc). From the same standpoint, if a class can store fewer member variables because they can be computed cheaply on demand, then it might very well be preferable to cheaply compute them on demand instead of storing and managing extra state. The less places that exist that have access to mutable states, the less potential there typically is for human error, and the fewer suspects we have to check when we encounter a bug caused from an invalid state (a failure to maintain an invariant). If we encounter a variable that is set to a value it should never have, then our immediate list of suspects is basically the number of lines that had access to the variable (the fewer the better). For this reason, it's likewise generally preferable not to create temporary local variables superfluously, like solely for the sake of aliasing/documentation with no other benefit, when a comment will do.

So in this ultra general, language-agnostic context, for any question that asks, "Should I create more mutable state than necessary?", I'd suggest to err on the side of less state to manage. A lot of the obstacles for a program to function correctly will be the number of mutable states multiplied by their scope. If you can get away with fewer variables or smaller scopes, then aim for fewer variables/smaller scopes.

Non-Generic Contexts

In cases where type information seems very relevant, including cases where the functions cannot hope to apply to more than one type of thing, it should generally be contextually obvious what this is. That's where I'd suggest looking to improve things more than trying to alias/document what this is in such a blatant and immediate way. That is, improve the documentation and organization of the surrounding code.

There are many ways to make such contextual info more obvious. For example, if the file name in which the function is defined is called cat.something, then it should be obvious that it applies to cat objects. If naming files to contain a single class is not idiomatic (something I found is not very idiomatic in Python, e.g., but it has special safeguards to make module-scope global variable access blatantly explicit to mitigate the problems), then perhaps we'll at least have something like this:

// Pretty obvious that this kills a cat.
cat.impale = new function(stick)
{
    this.kill()
    stick.combine(this);
}

If you're doing something fancy with closures as in Caleb's example where the difference between capturing this to a variable makes a logical difference and not merely designed for documentation, you'll notice he used the variable name, self, which is equally generic. That's also what I'd recommend if you encounter a scenario where it makes a difference: don't try to degeneralize the function by giving the variable capturing this a data type-specific name.

Asserting Preconditions

Yet let's say, after exhausting all of these scenarios, you still find the information about what this is both extremely relevant and elusive simultaneously (not obvious from the surrounding context). If refactoring is out of the question, then perhaps you could at least do something like this:

function impale(stick)
{
    assert(this is cat);
    this.kill();
    stick.combine(this);
}

With this at least, the assert does more than document. It guarantees that the function is invoked for a cat or else it'll become a runtime error, immediately noticeable (in a good way, helping to spot bugs immediately before they grow more difficult and elusive to correct the longer they fly under radar). I'd still recommend it as a last resort, but at least this does something more than just add documentation which could end up no longer matching what is actually going on. The assert makes it impossible: it serves as both documentation and an enforced guarantee/safeguard that the function will always match the expected behavior should it continue past the assertion.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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