Re: Clean up embedded scheme parsing/evaluation. (issue 577410045 by address@hidden)

[hanwenn]

On 2020/01/30 14:31:07, dan_faithful.be wrote:
> On Jan 30, 2020, at 04:54, mailto:address@hidden wrote:
> > 
> > This may predate you, but the decision to not store references was
> > intentional,
> > exactly because storing NULL in them is very useful.  If you have a
> > reference,
> > it has to be initialized in the constructor, and this introduces a
lot
> > of boilerplate
> > because you have to pass the non-null reference across constructors
in
> > the class
> > hierarchy. 
> 
> The discussion has turned from (a) passing a required parameter as a
reference,
> to (b) using a reference as a member variable.  (a) does not imply
(b).  You can
> pass in a T& and store it in a T* to avoid the constraints that (b)
would place
> on the use of your class (though they apparently were not previously a
problem
> in this case).
> 
> > The current code overwhelmingly disavows references. The 2 remaining
> > uses (this being one) stand out like a sore thumb.
> 
> We must be miscommunicating, because I see a lot more than 2.  For
some
> examples,

There are (were) 2 instance of non-const references passed in .hh files.
One dead method in Score_performer, and Lily_lexer::scan_word

In the lily/ directory

 git grep 'vector<[^>]\+> &' *c|grep -v const

returns 20 results, which is pretty small, given the number of methods
in the code base. A cursory inspection suggests that Mike introduced
most of these, and I would have probably suggested to use pointers there
too. I would also change these signatures if would stumble across them
while refactoring something else. Note that const& are OK as an
optimization of call by value; in the caller, the effect of cont& and
call by value is indistinguishable. 

I feel this whole discussion has gone out of hand, and in the interest
of expediency, I have replaced 

  const Input*

with

  Input

in the class declaration, so somebody can give this an LGTM now.

I also have the feeling we spend a lot of time and energy talking past
each other. How about a short voice/video call between the 3 of us to
make sure we are on the same page?

cheers,


> 
>     git grep 'vector<[^>]\+> &'
> 
> Please clarify.
> 
> Thanks,
> — 
> Dan
> 



https://codereview.appspot.com/577410045/