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 > > ~~~~~~~~~~~~~~~~~~ > >
