Re: Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-13 Thread Carl . D . Sorensen
On 2018/06/13 03:24:02, dan_faithful.be wrote: I perceive that we understand each other’s points and simply disagree. There is nothing new I want to counter with. I will just state that if a contributor were made uncomfortable by dynamic_cast, my two-pronged solution would be (1) gently

Re: Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-13 Thread Carl . D . Sorensen
I am convinced by these arguments. Thank you for your patience with me. Hopefully we can get some rats taken care of. Carl https://codereview.appspot.com/344010043/ ___ lilypond-devel mailing list lilypond-devel@gnu.org

Re: Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-13 Thread dak
On 2018/06/13 03:24:02, dan_faithful.be wrote: On Jun 12, 2018, at 18:18, mailto:carl.d.soren...@gmail.com wrote: > > The tradeoff of having people know about dynamic casting and using it > properly needs to be matched with people not needing to know about > dynamic casting and being able to

Re: Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-12 Thread Dan Eble
On Jun 12, 2018, at 18:18, carl.d.soren...@gmail.com wrote: > > The tradeoff of having people know about dynamic casting and using it > properly needs to be matched with people not needing to know about > dynamic casting and being able to ignore it. Carl, I appreciate hearing your perspective

Re: Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-12 Thread Carl . D . Sorensen
Dan, Thanks for the feedback. I appreciate it. I'm still not convinced that pulling the dynamic casts out of the getter/setter pair is better. You talk about performance penalties of dynamic casting. But your profiling shows that dynamic casting took about 1% of the processing time. Even if

Re: Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-12 Thread nine . fierce . ballads
I remembered another thing about dynamic casting which is not specifically relevant to the changes in this review, and which should be pretty well known, but I'll record it here for the sake of completeness. Wherever there is alternative processing predicated on type, e.g. if (A *a =

Re: Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-11 Thread nine . fierce . ballads
To summarize, the principle I advocate is to establish early what kind of object you are dealing with, and once you have paid for that information, retain it as long as you can. In LilyPond it seems I often see the opposite: casts in the lowest depths of the code. Regards, Dan

Re: Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-11 Thread nine . fierce . ballads
Reviewers: carl.d.sorensen_gmail.com, Message: My intent was to make it clear to the person using these objects what is going on. One thing that was hidden was the extra cost of getting an Item* over getting a Grob*. Consider this excerpt from the old version of Dynamic_align_engraver:

Issue 5336: Remove downcasting methods from Grob_array and Grob_info (issue 344010043 by nine.fierce.ball...@gmail.com)

2018-06-11 Thread Carl . D . Sorensen
I realize this issue has already been pushed and closed. But I have a question. What is the harm of having the downcasting in the getter function? An advantage is that we can change the downcasting in one place if it is part of the member functions for the class. If not, we have to change it