On Wed, 23 Jun 2021 10:38:43 +0200 (CEST) Fabien COELHO <coe...@cri.ensmp.fr> wrote:
> > Hello Yugo-san: > > # About v12.1 > > This is a refactoring patch, which creates a separate structure for > holding variables. This will become handy in the next patch. There is also > a benefit from a software engineering point of view, so it has merit on > its own. > ## Compilation > > Patch applies cleanly, compiles, global & local checks pass. > > ## About the code > > Fine. > > I'm wondering whether we could use "vars" instead of "variables" as a > struct field name and function parameter name, so that is is shorter and > more distinct from the type name "Variables". What do you think? The struct "Variables" has a field named "vars" which is an array of "Variable" type. I guess this is a reason why "variables" is used instead of "vars" as a name of "Variables" type variable so that we could know a variable's type is Variable or Variables. Also, in order to refer to the field, we would use vars->vars[vars->nvars] and there are nested "vars". Could this make a codereader confused? > ## About comments > > Remove the comment on enlargeVariables about "It is assumed …" the issue > of trying MAXINT vars is more than remote and is not worth mentioning. In > the same function, remove the comments about MARGIN, it is already on the > macro declaration, once is enough. Sure. I'll remove them. Regards, Yugo Nagata -- Yugo NAGATA <nag...@sraoss.co.jp>
