C++ StyleGuide: Using methods and public class/struct members #10

Falk David · 2024-01-22T14:19:58+01:00

Falk David commented

2024-01-22 14:19:58 +01:00

When using method or a public data member, use the this keyword.

When using method or a public data member, use the `this` keyword.

👍 4

Falk David added 1 commit 2024-01-22 14:20:00 +01:00

a013ac99c0 Using public class/struct members

Falk David requested review from Brecht Van Lommel 2024-01-22 14:20:34 +01:00

Falk David requested review from Jacques Lucke 2024-01-22 14:20:34 +01:00

Falk David requested review from Hans Goudey 2024-01-22 14:20:34 +01:00

Falk David changed title from ~~Using public class/struct members~~ to C++ StyleGuide: Using public class/struct members

2024-01-22 14:23:02 +01:00

Hans Goudey reviewed 2024-01-22 15:03:20 +01:00

Hans Goudey left a comment

Thanks for writing this up! In general I think it's important that there's context and the reader can easily see the scope of accessed functions and variables. So +1 for the proposal. One detail though-- I don't think this totally matches up with what we've been doing so far though. Another way to state it would be:

Use this-> prefix when accessing methods and public variables.

In other words, no need to use it for private variables with an _ suffix, where it's already clear they're part of the class.

Thanks for writing this up! In general I think it's important that there's context and the reader can easily see the scope of accessed functions and variables. So +1 for the proposal. One detail though-- I don't think this totally matches up with what we've been doing so far though. Another way to state it would be: > Use `this->` prefix when accessing methods and public variables. In other words, no need to use it for private variables with an `_` suffix, where it's already clear they're part of the class.

Falk David commented

2024-01-22 15:21:34 +01:00

@HooglyBoogly Hm I thought that my wording (and the title) made it clear that this is only for public members. Or maybe I'm not understanding what you mean?

Iliya Katushenock commented

2024-01-22 15:27:48 +01:00

This for any methods, not only for public.

Falk David commented

2024-01-22 15:30:33 +01:00

@mod_moder Ah yes, I see it now. Sorry, will fix.

Brecht Van Lommel approved these changes 2024-01-22 16:20:55 +01:00

Brecht Van Lommel left a comment

I'm fine with this.

For visibility, can you create a devtalk topic that links here? Then wait a few days and commit assuming there are no objections.

I'm fine with this. For visibility, can you create a devtalk topic that links here? Then wait a few days and commit assuming there are no objections.

Miguel Pozo commented

2024-01-22 16:20:59 +01:00

I personally find that having this-> everywhere adds unnecessary noise that hurts readability.
While I've never found it necessary (free functions are typically preceded by their module prefix/namespace, and global variables are rare), if someone wants the extra help for making the distinction, the solution is as easy as setting the syntax highlighting of your editor accordingly.

I personally find that having `this->` everywhere adds unnecessary noise that hurts readability. While I've never found it necessary (free functions are typically preceded by their module prefix/namespace, and global variables are rare), if someone wants the extra help for making the distinction, the solution is as easy as setting the syntax highlighting of your editor accordingly.

Bastien Montagne commented

2024-01-22 16:43:21 +01:00

I would also prefer not to see this enforced. I find it adding quite some noise to the code, and it should not be needed in most cases.

But I could live with this rule if most other devs think it's a good one. ;)

I would also prefer not to see this enforced. I find it adding quite some noise to the code, and it should not be needed in most cases. But I could live with this rule if most other devs think it's a good one. ;)

Falk David added 1 commit 2024-01-22 16:48:36 +01:00

8bcf1d7f15 Update phrasing

Iliya Katushenock commented

2024-01-22 16:51:22 +01:00

the solution is as easy as setting the syntax highlighting of your editor accordingly

Not all editor support such functionality. And even more, i'd prefer to do not totally rely on parsing of includes for anything if this is not necessary.

> the solution is as easy as setting the syntax highlighting of your editor accordingly Not all editor support such functionality. And even more, i'd prefer to do not totally rely on parsing of includes for anything if this is not necessary.

Falk David commented

2024-01-22 16:58:27 +01:00

@brecht Created a discussion thread: https://devtalk.blender.org/t/c-style-guide-using-the-this-keyword-when-accessing-public-members-and-methods/33029

Jacques Lucke commented

2024-01-22 17:20:35 +01:00

I definitely prefer using this-> because otherwise it's unclear whether the called function depends on this or not. I quite often found myself in code by others where I wondered whether something is calling a standalone function or something that depends on this. Even Python enforces this, which is otherwise a very concise language.

I haven't seen this add a significant amount of noise. The only places I can think of where this is annoying is when writing a somewhat complex math expression that uses many data members with short names. Overall, this seems rare in most code. If it's really a problem in some function there are easy ways around it: make the data members private and use the trailing underscore or just making a local copy of the variable.

I definitely prefer using `this->` because otherwise it's unclear whether the called function depends on `this` or not. I quite often found myself in code by others where I wondered whether something is calling a standalone function or something that depends on `this`. Even Python enforces this, which is otherwise a very concise language. I haven't seen this add a significant amount of noise. The only places I can think of where this is annoying is when writing a somewhat complex math expression that uses many data members with short names. Overall, this seems rare in most code. If it's really a problem in some function there are easy ways around it: make the data members private and use the trailing underscore or just making a local copy of the variable.

Jacques Lucke approved these changes 2024-01-22 17:20:42 +01:00

Hans Goudey approved these changes 2024-01-22 17:24:17 +01:00

Bastien Montagne commented

2024-01-22 17:38:34 +01:00

Shouldn't calls to private/protected methods also use this-> actually?

class X {
 public:
  int my_int;
 
 public:
  void foo() {
    /* bar(); */
    this->bar();
  }
 
 private:
  void bar() {
    this->my_int = 42;
  }
};

EDIT: Re-reading the current proposed text, this is what it says now I think... But then maybe the example can be updated to show also the call to a private method?

Shouldn't calls to private/protected methods also use `this->` actually? ``` c++ class X { public: int my_int; public: void foo() { /* bar(); */ this->bar(); } private: void bar() { this->my_int = 42; } }; ``` EDIT: Re-reading the current proposed text, this is what it says now I think... But then maybe the example can be updated to show also the call to a private method?

Falk David commented

2024-01-22 17:46:47 +01:00

@mont29 Yes I can add a few more examples and cover more cases :)

👍 1

Falk David added 1 commit 2024-01-22 18:20:52 +01:00

a046224406 Add more examples

Falk David changed title from ~~C++ StyleGuide: Using public class/struct members~~ to C++ StyleGuide: Using methods and public class/struct members

2024-01-22 18:21:41 +01:00

Bastien Montagne approved these changes 2024-01-22 18:30:29 +01:00

Falk David commented

2024-01-23 10:22:11 +01:00

I'll wait till the end of the week, so others have a chance of seeing the devtalk topic and give their feedback.

Falk David commented

2024-01-23 10:24:28 +01:00

One question: Should I add an example for struct as well?

struct X {
  int my_int;
 
  void foo() {
    this->bar();
  }
 
  void bar() {
    this->my_int = 42;
  }
};

One question: Should I add an example for `struct` as well? ``` struct X { int my_int; void foo() { this->bar(); } void bar() { this->my_int = 42; } }; ```

Jacques Lucke commented

2024-01-23 10:26:56 +01:00

To me the important thing is that this-> is used when there is no trailing underscore in the member name. Not sure if it really helps to have even more examples. Makes it look more complex than it is.

To me the important thing is that `this->` is used when there is no trailing underscore in the member name. Not sure if it really helps to have even more examples. Makes it look more complex than it is.

👍 3

Jacques Lucke reviewed 2024-01-24 14:42:09 +01:00

docs/handbook/guidelines/c_cpp.md Outdated

						
				@ -735,6 +735,39 @@ class X {

				};

				```

				## Using public class/struct data members and methods

Jacques Lucke commented

2024-01-24 14:35:05 +01:00

Maybe call this "Use of this->"

Falk David commented

2024-01-25 11:01:09 +01:00

To me this is not so much about the general use of this though. Or do you literally mean this-> ? I think it's better to be explicit.
Maybe "Accessing methods and public data members in classes and structs" ?

To me this is not so much about the general use of `this` though. Or do you literally mean `this->` ? I think it's better to be explicit. Maybe "Accessing methods and public data members in classes and structs" ?

Jacques Lucke commented

2024-01-25 14:32:44 +01:00

I'm mostly just trying to make this guide useful for people who don't know already what's in it. People might go to it because they want to know whether they should use this-> or not, not because they don't know how to access methods and data members. Don't have a strong opinion on this though.

I'm mostly just trying to make this guide useful for people who don't know already what's in it. People might go to it because they want to know whether they should use `this->` or not, not because they don't know how to access methods and data members. Don't have a strong opinion on this though.

Hans Goudey commented

2024-01-25 14:36:11 +01:00

Agreed with Jacques here personally. Mentioning this-> in the title of this section is the simplest way to tell people what its point is.

Agreed with Jacques here personally. Mentioning `this->` in the title of this section is the simplest way to tell people what its point is.

filedescriptor marked this conversation as resolved

docs/handbook/guidelines/c_cpp.md Outdated

						
				@ -737,1 +737,4 @@

				## Using public class/struct data members and methods

				Use `this->` prefix when accessing methods and public data members.

Jacques Lucke commented

2024-01-24 14:36:07 +01:00

"Use this-> when accessing methods and data members that don't have a [trailing underscore](link-to-that-guide)."

Think that phrasing results in less confusion.

"Use `this->` when accessing methods and data members that don't have a `[trailing underscore](link-to-that-guide)`." Think that phrasing results in less confusion.

Falk David commented

2024-01-25 11:07:06 +01:00

Seems a bit odd to me to use the trailing underscore as the primary reasoning. This would read better to me:
"Use this-> when accessing methods and public data members (i.e. members that don't have a [trailing underscore](#class-data-member-names))."

Seems a bit odd to me to use the trailing underscore as the primary reasoning. This would read better to me: "Use `this->` when accessing methods and public data members (i.e. members that don't have a `[trailing underscore](#class-data-member-names)`)."

Jacques Lucke commented

2024-01-29 09:37:14 +01:00

Hm, I thought I wrote a comment here earlier, but somehow it is gone. I'm not using the trailing underscore as a reason but as the rule/guide. The reason is that I want to be able to see whether something depends on this or not. With the trailing underscore it is already apparent, so the extra this-> is not necessary.

If, hypothetically, we had a rule like all class members (data and methods) had to use a trailing underscore, we wouldn't need this-> IMO. So there is definitely an important relation.

Hm, I thought I wrote a comment here earlier, but somehow it is gone. I'm not using the trailing underscore as a *reason* but as the *rule/guide*. The reason is that I want to be able to see whether something depends on `this` or not. With the trailing underscore it is already apparent, so the extra `this->` is not necessary. If, hypothetically, we had a rule like all class members (data and methods) had to use a trailing underscore, we wouldn't need `this->` IMO. So there is definitely an important relation.

filedescriptor marked this conversation as resolved

docs/handbook/guidelines/c_cpp.md

						
				@ -738,0 +740,4 @@

				Use `this->` prefix when accessing methods and public data members.

				``` c++

				class X {

Jacques Lucke commented

2024-01-24 14:38:53 +01:00

Simplified example:

class X {
 private:
  float my_float_;

 public:
  int my_int;

  void foo() {
    /* Use `this->` because there is no trailing underscore. */
    this->my_int = 42;
    this->bar();

    /* Do *not* use `this->` because there is a trailing underscore. */
    my_float_ = 3.14f;
  }

  void bar() {}
}

Simplified example: ```cpp class X { private: float my_float_; public: int my_int; void foo() { /* Use `this->` because there is no trailing underscore. */ this->my_int = 42; this->bar(); /* Do *not* use `this->` because there is a trailing underscore. */ my_float_ = 3.14f; } void bar() {} } ```

👍 1

Falk David commented

2024-01-25 11:09:15 +01:00

Again I'm not sure I would use the trailing underscore as the reasoning for using this. That raises more questions in my mind :D But maybe others can comment on this? @brecht @Sergey ?

Again I'm not sure I would use the trailing underscore as the reasoning for using `this`. That raises more questions in my mind :D But maybe others can comment on this? @brecht @Sergey ?

Jacques Lucke commented

2024-01-25 14:35:09 +01:00

The trailing underscore is not the reason, it is the rule/guide.

The reason is that we want to be able to see whether a specific symbol belongs to a class/struct or not. When there is a trailing underscore, that is already enough information.

If, hypothetically, we had a rule that would force us to use trailing underscores for all members, then we wouldn't have to use this-> imo. Obviously, we don't and shouldn't have this rule.

The trailing underscore is *not* the *reason*, it is the rule/guide. The reason is that we want to be able to see whether a specific symbol belongs to a class/struct or not. When there is a trailing underscore, that is already enough information. If, hypothetically, we had a rule that would force us to use trailing underscores for all members, then we wouldn't have to use `this->` imo. Obviously, we don't and shouldn't have this rule.

filedescriptor marked this conversation as resolved

docs/handbook/guidelines/c_cpp.md Outdated

						
				@ -738,0 +765,4 @@

				};

				```

				Private and protected members don't need the explcit `this->`

Jacques Lucke commented

2024-01-24 14:41:07 +01:00

This part can be removed with the proposed sentence above. Also there is a typo in explicit.

This part can be removed with the proposed sentence above. Also there is a typo in `explicit`.

👍 1

filedescriptor marked this conversation as resolved

Jacques Lucke commented

2024-01-24 14:42:47 +01:00

Added some comments for how I'd phrase this style guide.

Falk David added 1 commit 2024-01-29 11:49:22 +01:00

e6d20f128e Update phrasing

Falk David added 1 commit 2024-01-29 11:50:35 +01:00

946b7c2b38 Remove unecessary sentence

Falk David requested review from Hans Goudey 2024-01-29 11:51:05 +01:00

Falk David requested review from Jacques Lucke 2024-01-29 11:51:05 +01:00

Falk David requested review from Brecht Van Lommel 2024-01-29 11:51:06 +01:00

Jacques Lucke approved these changes 2024-01-29 12:02:05 +01:00

Hans Goudey approved these changes 2024-01-29 14:23:54 +01:00

Brecht Van Lommel approved these changes 2024-02-01 16:40:39 +01:00

Falk David merged commit 527bb46ad4 into main

2024-02-01 16:51:01 +01:00

Falk David deleted branch using-this-keyword

2024-02-01 16:51:02 +01:00

Falk David referenced this issue from a commit

2024-02-01 16:51:07 +01:00

C++ StyleGuide: Using `this->` (#10)

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

7 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender-developer-docs#10

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

C++ StyleGuide: Using methods and public class/struct members #10