[Bf-committers] Newbie point of view: variable declaration and readability

Campbell Barton ideasman42 at gmail.com
Wed May 9 07:42:54 CEST 2018


Hi Christian,

Please leave code in-place where possible and avoid the urge to
*cleanup* at the same time as porting existing code.

If you think a cleanup is useful, submit this as a separate diff.
This means if there are any regressions we need to investigate in the
future (which seems likely given the amount of code), we can more
easily track this back to either the port or to some other changes.

It also means we don't have to get into discussions about
conventions/style - when we can better spend that time getting things
working again.

If you check Blender's commit history, we do cleanups as part of
general development, so there is no reason to think it will never
happen if it's not done now.




On Tue, May 8, 2018 at 8:26 PM, Christian Hubert
<christian.hubert at dstribe.com> wrote:
> Hi,
>
>
>
>
>
> First, sorry for this long email. I know 2.8 is the nowadays focus, but I
> cannot be a newbie twice (or.).
>
>
>
> During a port to 2.8, I was asked to keep in place some variables
> declarations (Sybren, nothing personal about that, it is just to understand
> what should be done or not as this is not mentioned in code style).
>
>
>
> So I did the port as wanted (at least, I hope).
>
>
>
> But my though is "this is not good". Here are the reasons why:
>
>
>
> A very simple example (original code):
>
>
>
> <start of the example >
>
>     DerivedMesh *dm_other;
>
>
>
>     if (!bmd->object)
>
>         return dm;
>
>
>
>     dm_other = get_dm_for_modifier(bmd->object, ctx->flag);
>
> <end of the example >
>
>
>
> We have a " dm_other " floating in the air and which could or not be
> used/instantiated later.
>
>
>
> If we want to tell a story that could be:
>
>
>
> *       I want to tell you about a DerivedMesh which is called dm_other
> *       Oh. but wait. Anyway if my data does not know an object I quit
> *       Oh. but if not, dm_other is now a nice derived mesh
>
>
>
> mmm.
>
>
>
> If we rewrite that:
>
>
>
> <start of the example >
>
>     if (!bmd->object)
>
>         return dm;
>
>
>
>     DerivedMesh *dm_other = get_dm_for_modifier(bmd->object, ctx->flag);
>
> <end of the example >
>
>
>
> The story is now:
>
>
>
> *       Anyway we need to have an object or we quit
> *       But as we stay there, here is the story of a derived mesh called
> dm_other which is a nice derived mesh
>
>
>
> Well, some could think this makes few difference. But:
>
>
>
> *       In larger code you need to remember variable names and types which
> are declared previously and used/instanciated (or not) much later
> *       When they are instanciated, knowing their types is not always
> obvious
> *       Code is more verbose, divided
> *       Direct comprehension and maintenance is harder
> *       Eventually some code parts won't be run for some reasons/conditions
> and these declarations will be useless
> *       Variable instantiation/initializaition is not guaranteed
>
>
>
> So. maybe what I'm saying here is not so important. Anyway the code will run
> and do the job. But I really believe this is part of code quality (as
> nowadays compiler accept that instead of declaring everything at the
> beginning of each function) and we need to remember that "a code is written
> once, read very often, modified sometimes".
>
>
>
> And I also believe this kind of big and great project (I mean Blender)
> depends only on people will and all that leads to the straight (more simple)
> way to understand and do things, the more effective participants will be
> (sorry for my bad english, I hope this is understandable).
>
>
>
>
>
> So now, If we come back to the 2.8 port:
>
>
>
> OK the port needs to be as straight as possible: we don't want side effects
> in these circumstances.
>
>
>
> But in the other hand, if we don't change now, we will never change. or is
> there "code quality only" (not features bound) revision steps? I do not
> know.
>
>
>
>
>
> Well. I hope this email can be constructive. If it is not from your
> feedback, I'll stop this kind of things. But, if it is considered
> constructive, I will post more about my newbie first steps in this
> (fantastic but so hard) new world.
>
>
>
> If you want more, the next "newbie point of view" could be: "Which API or
> API level should I use?"
>
>
>
>
>
> Kind greetings,
>
> Christian
>
>
>
>
>
> _______________________________________________
> Bf-committers mailing list
> Bf-committers at blender.org
> https://lists.blender.org/mailman/listinfo/bf-committers



-- 
- Campbell


More information about the Bf-committers mailing list