On Wed, 23 Jun 2021 10:38:43 +0200 (CEST)
Fabien COELHO <coelho@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 <nagata@sraoss.co.jp>
