Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistent vertical spacing for access modifiers in [ranges] #4704

Open
tkoeppe opened this issue Jun 17, 2021 · 11 comments
Open

Consistent vertical spacing for access modifiers in [ranges] #4704

tkoeppe opened this issue Jun 17, 2021 · 11 comments

Comments

@tkoeppe
Copy link
Contributor

tkoeppe commented Jun 17, 2021

We currently often don't have any blank lines between private: and public: parts, especially in [ranges].

We're also inconsistent between class X { stuff; and class X { private: stuff;.

I propose the following, to be added to the editorial wiki:

  • Always make access specifiers explicit when the class key is class. If a private section is needed at the beginning, that means we say class { private:.
  • Always separate access sections with a blank line.
@CaseyCarter
Copy link
Contributor

  • Always make access specifiers explicit when the class key is class. If a private section is needed at the beginning, that means we say class { private:.

FWIW, I find this inconsistency quite annoying: do we really believe that a significant proportion of readers understand that struct's default access is public but not that class's default access is private? It's one of only two differences between them.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 17, 2021

@CaseyCarter: Do you mean the inconsistent ways we use class, or the inconsistency of making private explicit for class but not making public explicit for struct?

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 17, 2021

The way I see it is that predominantly the Standard is about the public interfaces, so we present the public parts "by default": that means no explicit specifers for struct (because that's the common case), and public: for class because we have to. By contrast, private sections are always worth calling out and separating explicitly, because they're not part of the observable interface. Ideally we can keep the private parts at the end of each class definition, but if it must be at the beginning, then I'd still like to retain the explicit specifier, and not lean on the default here.

@jensmaurer
Copy link
Member

It's a small nit not worth fighting over, but I think class X { private: ... just asks for frowny faces all around. Even if you overlook the "private by default" situation for classes, the stuff that follows is all "exposition only" and "kebab-case", so hardly a meaningful part of a public specification.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 17, 2021

We also sometimes have structs that start with private: (by necessity of course). So my proposal would be "always explicit except for struct-public.

Some of the new range view classes have very verbose private sections, and I'm even tempted to try out more internal vertical whitespace, e.g. newlines between items which each have long comments, long requires clauses, and long comments like "enabled only if Foo". It's getting very dense and crowded.

Once you imagine taking up more space overall, explicit section markers might be useful for quick orientation.

I'm ultimately not decided between either style yet, but do we agree at least that we should pick one?

@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Jun 17, 2021
@jensmaurer
Copy link
Member

Consistency is good.

For starters, I think "structs with private members" should be turned into classes right away.

And, if we're not doing this already, an empty line before an access-specifier is a good idea.

Also, moving the "exposition only" comment more to the right also de-crowds the presentation.

@CaseyCarter
Copy link
Contributor

@CaseyCarter: Do you mean the inconsistent ways we use class, or the inconsistency of making private explicit for class but not making public explicit for struct?

The latter.

The way I see it is that predominantly the Standard is about the public interfaces

Here we're going to disagree. To me, the standard library specification is primarily concerned with behavior; how things act is more important than how they look. The specifications of the public and private (or exposition-only, which should be synonymous) parts of a given class are equally important for conveying the intended behavior. Your argument is a good one for the content of header files in an actual C++ project which the standard's synopses are not: we don't specify truly private implementation details at all. That said, I've expressed my dissenting opinion and I'm now happy to stop arguing the issue =)

I'm ultimately not decided between either style yet, but do we agree at least that we should pick one?

Yes, absolutely. I wish I'd understood how the library specifies things and appreciated the value consistency in large projects as much then as I do now.

Also, moving the "exposition only" comment more to the right also de-crowds the presentation.

It seems silly that we have to comment every private member of every standard library class with // \expos. Could we simply add blanket wording to the effect that all depicted private members are for exposition only?

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 17, 2021

One point in possible favour of explicit private:: Some of the class heads (e.g. split_view) are very long, and the private: provides a vertical buffer, without which the first member would go right up against the dense class head. Is that worth something?

image
vs.

image

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jun 17, 2021

Re blanket wording / expos comments: That's something we could certainly consider.

@jensmaurer
Copy link
Member

This is a slightly inopportune moment to discuss this. I'd much prefer if we'd apply any pending editorial fixes that are low-risk and can go in before the mailing, and then ship the updated Working Draft.

The discussion here needs some ideas, thinking, and discussion, none of which are on the plate right now.

@jensmaurer
Copy link
Member

Editorial meeting 2021-06-18: Move all private parts (in particular non-static data members) to the bottom as much as possible. Maybe all that remains are nested class types with a nice "// cross-ref" introducer.

Leave the rest as-is for now.

@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants