Hi Julian Mainly I agree with less code is more readable, but over less is also hard to read for the beginner.
Regards! Aron Tao Chunwei Lei <[email protected]> 于2020年11月25日周三 下午3:54写道： > I have the same feeling. But I am +0 on this change considering: > 1) we can not forbid users to write such code. > 2) there might be other codes that are not debug-friendly and we can not > change them all. > > > Best, > Chunwei > > > On Tue, Nov 24, 2020 at 10:59 AM JiaTao Tao <[email protected]> wrote: > > > Hi James > > My point is not all intermediate variables, please don't expand the > scope, > > you may not family with "relBuilder.build()", you can not run this method > > twice, it's not idempotent. > > > > I'm usually using "option+click" to direct see the expression's value, > it's > > so convenient, but RelBuilder#peek is a good way. > > > > Regards! > > > > Aron Tao > > > > > > James Starr <[email protected]> 于2020年11月24日周二 上午6:38写道： > > > > > I second readability over debug ability. If every function call is > > > decompressed to its own variable, readability can be compromised while > > make > > > it more annoy to debug because there are now so many > > > intermediate variables. Calcite compiles relatively quickly, it is not > > too > > > inconvenient to refactor a relevant portion of code to make debugging > > for a > > > particular task easier, then recompile, before refactoring for > > > readability/clarity at the end. > > > > > > James > > > > > > On Mon, Nov 23, 2020 at 11:49 AM Julian Hyde <[email protected]> > > > wrote: > > > > > > > I am -0 on this change. > > > > > > > > Most code is read many more times than it is debugged. If you expand > > code > > > > so that it takes more screen area, the reader has to read more code > in > > > > order to figure out what’s going on. > > > > > > > > Of course you can take that philosophy too far. If I have a > multi-line > > > > expression, with large nested calls, it’s almost always worth > creating > > > > intermediate variables for those calls. > > > > > > > > In the case of RelBuilder.build(), I happen to know that it is > > basically > > > > popping the top element off RelBuilder’s stack, so I would take a > look > > at > > > > that stack rather than stepping in. > > > > > > > > Recent versions of Intellij allow you to choose which function you > step > > > > into. If you are on the line > > > > > > > > call.transformTo(relBuilder.build()); > > > > > > > > and press f7 (step into), Intellij will ask you highlight > ’transformTo’ > > > > and ‘build’ and let you choose one. > > > > > > > > By the way, some folks like to assign values to variables before they > > > > return them, e.g. > > > > > > > > final RelNode r = relBuilder.build(); > > > > return r; > > > > > > > > I am not fond of that pattern either. > > > > > > > > Julian > > > > > > > > > > > > > > > > > On Nov 23, 2020, at 6:32 AM, Albert <[email protected]> wrote: > > > > > > > > > > +1, `step into` causes bad debug experience. > > > > > > > > > > On Mon, Nov 23, 2020 at 6:13 PM JiaTao Tao <[email protected]> > > > wrote: > > > > > > > > > >> I can create a JIRA and update the code, it's minor but I think it > > is > > > > good > > > > >> for us. > > > > >> > > > > >> > > > > >> Regards! > > > > >> > > > > >> Aron Tao > > > > >> > > > > >> > > > > >> Haisheng Yuan <[email protected]> 于2020年11月23日周一 下午5:53写道： > > > > >> > > > > >>> Agree with Jiatao, I had the same experience and feeling. But it > > > mainly > > > > >>> depends on the rule creator's preference. > > > > >>> > > > > >>> On 2020/11/23 02:42:21, Danny Chan <[email protected]> wrote: > > > > >>>> I kind of agree, but it's more like a programming specification, > > we > > > > can > > > > >>>> tell people how to write codes but they may not follow those > > rules. > > > > >>>> > > > > >>>> JiaTao Tao <[email protected]> 于2020年11月22日周日 下午5:27写道： > > > > >>>> > > > > >>>>> Why I don't want to debug into "transformTo": > > > > >>>>> > > > > >>>>> 1. It's a common method, if you directly stop here, every rule > > will > > > > >>> stop, > > > > >>>>> or you must stop the specific rule, then step into this method > > > call, > > > > >>> it's > > > > >>>>> one more step. > > > > >>>>> 2. There are many contexts in the rule, if you debug into > > > > >>> "transformTo", > > > > >>>>> you have to go back to see these. > > > > >>>>> > > > > >>>>> > > > > >>>>> Regards! > > > > >>>>> > > > > >>>>> Aron Tao > > > > >>>>> > > > > >>>>> > > > > >>>>> JiaTao Tao <[email protected]> 于2020年11月22日周日 下午5:23写道： > > > > >>>>> > > > > >>>>>> Hi > > > > >>>>>> I've been developed Calcite full time for a quite long time, > > and I > > > > >>> ofter > > > > >>>>>> debug in the rule to see the transformations, but code like > this > > > is > > > > >>> not > > > > >>>>>> debuging friendly in my opinion: > > > > >>> "call.transformTo(relBuilder.build())" > > > > >>>>>> > > > > >>>>>> I want to see the relBuilder.build()'s result, I have to debug > > > into > > > > >>> the > > > > >>>>>> "transformTo" method(you can not evaluate "relBuilder.build()" > > cuz > > > > >>> it's a > > > > >>>>>> stack), if we split this into two lines, we can just stop at > the > > > > >> last > > > > >>>>> link: > > > > >>>>>> > > > > >>>>>> RelNode ret = relBuilder.build() > > > > >>>>>> call.transformTo(ret) > > > > >>>>>> > > > > >>>>>> It's not a big deal, but every time I occur this, it has poor > > > > >>>>> experience, hope > > > > >>>>>> to hear the community's opinion. > > > > >>>>>> > > > > >>>>>> Regards! > > > > >>>>>> > > > > >>>>>> Aron Tao > > > > >>>>>> > > > > >>>>> > > > > >>>> > > > > >>> > > > > >> > > > > > > > > > > > > > > > -- > > > > > ~~~~~~~~~~~~~~~ > > > > > no mistakes > > > > > ~~~~~~~~~~~~~~~~~~ > > > > > > > > > > > > > >
