[R] Unnecesary code?

[Duncan Murdoch]

On 19/11/2009 4:23 AM, Hun S. Tesatte wrote:
> 
> On Thu, 19 Nov 2009 00:13:27 +0100 Duncan Murdoch 
> <murdoch at stats.uwo.ca> wrote:
>> hunsyntesat at hush.com wrote:
>>> Dear R-ers,
>>>
>>> While browsing the R sources, I found the following piece of 
>> code 
>>> in src\main\memory.c:
>>>
>>> static void reset_pp_stack(void *data)
>>> {
>>>     R_size_t *poldpps = data;
>>>     R_PPStackSize =  *poldpps;
>>> }
>>>
>>> To me, it looks like the poldpps pointer is a nuissance; can't 
>> you 
>>> just cast the data pointer and derefer it at once? Say,
>>>
>>> static void reset_pp_stack(void *data)
>>> {
>>>     R_PPStackSize = * (R_size_t *) data;
>>> }
>>>   
>> What would you gain by this change?
>>
>> Duncan Murdoch
> 
> Seriously? What would you gain by rejecting the change?

I would save about an hour spent making the change, testing and 
committing it.

> I think the gain is obvious, even if not essential: the code is 
> cleaner. If there is a choice between two different pieces of code 
> that have the same effect, choosing the simpler makes it easier to 
> maintain the code, and easier for a casual user to understand 
> what's going on. Anyone looking at the original code for the first
> time will have to realise that poldpps is a nuissance variable with 
> no practical importance and no gain whatsoever, the change cuts 
> this need.  

But it makes the expression more complex, and doesn't give a hint about 
what's going on.  The name poldpps adds a bit of explanation of what the 
assumption is about what's being passed in data.

> 
> There is also a negligible loss in performance when the inessential 
> stack variable is allocated.

There is likely no variable allocated.  Compilers are reasonably smart 
these days.

Duncan Murdoch

> 
> -- Hun