Bug 39663 - Update the style guide re: static member variables and structs
Summary: Update the style guide re: static member variables and structs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-25 02:07 PDT by Jeremy Orlow
Modified: 2010-05-28 02:02 PDT (History)
2 users (show)

See Also:

Attachments
Patch (2.07 KB, patch)
2010-05-25 02:14 PDT, Jeremy Orlow 		no flags Details | Formatted Diff | Diff
Patch (1.59 KB, patch)
2010-05-26 03:45 PDT, Jeremy Orlow 		no flags Details | Formatted Diff | Diff
Patch (1.59 KB, patch)
2010-05-27 06:23 PDT, Jeremy Orlow 		darin: review+
Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2010-05-25 02:07:19 PDT 
Update the style guide re: static member variables and structs
Comment 1 Jeremy Orlow 2010-05-25 02:14:37 PDT 
Created attachment 56995 [details]
Patch
Comment 2 Darin Adler 2010-05-25 08:55:15 PDT 
Comment on attachment 56995 [details]
Patch

> -<li>Prefix C++ data members with "m_".
> +<li>Prefix C++ class member variables with "m_" and do not make them public.

The term "data members" is correct C++ terminology. The term "member variables" is not correct C++ terminology. I would write:

    Prefix C++ data members with "m_" and static data members with "s_" and do not make them public

> +<li>Static member variables should be prefixed with "s_".</li>

The correct term is "static data members".
Comment 3 Jeremy Orlow 2010-05-25 09:09:41 PDT 
(In reply to comment #2)
> (From update of attachment 56995 [details])
> > -<li>Prefix C++ data members with "m_".
> > +<li>Prefix C++ class member variables with "m_" and do not make them public.
> 
> The term "data members" is correct C++ terminology. The term "member variables" is not correct C++ terminology. I would write:
> 
>     Prefix C++ data members with "m_" and static data members with "s_" and do not make them public

Does "C++ data member" imply class (vs. structs)?  Even if so, I think we should explicitly explain that public data members are OK with structs but not classes.
 
> > +<li>Static member variables should be prefixed with "s_".</li>
> 
> The correct term is "static data members".

I will delete this and mention it above instead.
Comment 4 Kent Tamura 2010-05-26 03:20:51 PDT 
(In reply to comment #3)
> Does "C++ data member" imply class (vs. structs)?  Even if so, I think we should explicitly explain that public data members are OK with structs but not classes.

"data member" is used for structs too in the standard.
Comment 5 Jeremy Orlow 2010-05-26 03:45:04 PDT 
Created attachment 57088 [details]
Patch
Comment 6 Darin Adler 2010-05-26 10:49:47 PDT 
Comment on attachment 57088 [details]
Patch

> -<li>Prefix C++ data members with "m_".
> +<li>All data members in C++ classes must be private.  Static data members should be prefixed by "s_".  All other data members should be prefixed by "m_".

Why the "must" vs. "should" here? I don’t think we need the word “All”.
Comment 7 Jeremy Orlow 2010-05-27 06:23:18 PDT 
Created attachment 57233 [details]
Patch
Comment 8 Jeremy Orlow 2010-05-28 02:02:15 PDT 
Committed r60346: <http://trac.webkit.org/changeset/60346>