Re: Stylistic changes and documentation.

[al davis]

On Thu, 24 Nov 2022 12:34:43 +0100
Felix Salfelder <felix@salfelder.org> wrote:
> 1) auto. While generally not a good idea (obfuscates code too quickly),

After having it available for over 10 years now, and seeing lots of use
of it, I still think it is generally not a good idea.

Working with some item, looking at its declaration, often the first
thing I want to know about it is its type.   "auto" makes me look
elsewhere for it.  I would rather have it explicitly there.

If there is real benefit in its use, I have no objection to it, but I
do object to overuse where there is no benefit, or more so where it
makes me look for the type, or the type is obscure.   The examples
given are all of the type where I see no benefit of auto. but since the
type is explicit on that line, it isn't terrible.

One spot where auto does significantly obfuscate is the range-based
loop.

        for (auto bob : box) {
          //
        }


This brings up another item for the list ...

6) The range-based loop ......

It could be good, or not.  

One point about it is that this form does not explicitly specify the
order of the iteration.  Therefore I would only use it in cases where
that is the intent, and even in that case, explicitly specify the type.

int main() {
  int numberArray[] = {
    100,
    200,
    300,
    400,
    500
  };
  for (int number: numberArray) {
    cout << number << " ";
  }
  return 0;
}

prints: 100 200 300 400 500 
....  or does it?
why not: 300 200 500 400 100
....  ?

We really need a loop like this, or better yet .. one that supports
parallel operation.  This would have been a good place to introduce it.





> 2) override. Functions intended to override others should be marked as
>    such. This will improve readability as well as catch typos and type
>       mismatch immediately.

Yes.   Use it.  I think it is unfortunate that its use is not required.
It seems like somebody did half of the job.

  class A {
    virtual void x() {}
    virtual void y() {}
  };

  class B : public A {
    void x() override {}      // good
    void y()          {}      // accepted???? override optional
    void x(int i) override {} // rejected, as intended
    void y(int i)          {} // accepted, is it intended or not?
    void x(double d) {}       // not sure what happens here.
    void y(double d) {}       // accepted, as it always was.
  };

> 3) delete. It is easier to delete unwanted defaults than implementing
>    dummies.

Yes.  Use it.  

The dummies do work as intended, with intended errors at compile time.
It's nice to have a real language feature here, to not need to resort
to dummies.

There is still a hole.  If you don't either delete or make a dummy, you
can still get caught.

> 4) Private data members. In some (presumably old?) code they are at the
>    top of the class.  I believe they should always be at the bottom. They
>    are not very interesting or insightful when looking at the interface.

Having tried it both ways, and not complaining, I really think they
should be on top.  Gnucap code is mostly on top, but some on bottom,
which probably should be moved to top.

Why is it the way it is, historically????    In the early days, they
had to be on top.  Single pass compiler requires to declare everything
before first use.  Then it was ok at bottom too.  I thought the same
thoughts as yours, did a bunch that way.  In time, discovered that I
really didn't like it.

Often the internal structure including private data is important to the
caller.  Example:  parameters.

There is also the notion that scope as defined by C++ may not be the
same as the scope to a human.  Example: a "class" with some methods
implemented externally, just below in the same file.  To a human
reader, it's all the same scope.  To C++, it isn't.  So, putting the
data at the bottom of the "class", it's in the middle when the human
reader tries to find it, not at all where I want it to be.




> 5) New code must apply to style rules. Trivial style Changes in existing
>    code have to be be committed seperately. We should actually do this
>    sooner than later esp. in regions where changes are expected.

I agree with this.  Don't mix these changes with other stuff.  Make an
explicit branch for it, then merge to develop, like we do for feature
branches.

> 
> Do you agree with these? There are proably more. Let me know, I will add
> them to [1].