kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

On 04/29/2012 11:05 AM, Lorenzo Marcantonio wrote:
> On Sun, Apr 29, 2012 at 09:06:07AM -0500, Dick Hollenbeck wrote:
>> I don't know if you even want feedback.  This nice work.
> Feedback is always good.
>
>> I do have some minor points here, and then search for my initials below "RHH".
>>
>>
>> 1) Since it was such a significant revision, would have been nice to get the code more in
>> line with current coding standards, such as a) uppercase public function names, b) single
>> line comments C++ sytle, c) function comments in the header file, not in the C++ file. 
>> Remember that function comments are inherited by Doxygen into a polymorphic derivative
>> function.  So they only need to go into the base class within the header file unless there
>> is something different in a derivative.  Doxygen scans for functions in header files as a
>> preference to C++, so that is our preference.
> OK. Didn't knew about the doxygen bit.
>
>> 2) There were a couple of places where you removed KiROUND() when converting from double
>> to int.  Maybe you wanted a different rounding algorith, but you also removed
>> overflow/underflow detection which KiROUND() provides.
> I'll look into them. Didn't know about that KiROUND feature, too.
>
>> Of the two, I like b) better, and of course even better yet would be upper case first
>> character function name.
>  
> Returning a DPOINT conses ...erhmm... creates and destroys the object, a pointer or a reference doesn't;


So we think the printer, disk, or plotter is going to push the bottleneck onto the code
behind this choice?  :)
You know, by being really really fast printers, disks, or plotters?


If this was Java, I would agree with you, since you cannot instantiate anything without
"new"ing it.  And once you've paid for that heap operation, then by all means it is faster
to reuse that object with a reference.


But this not Java, and we are not talking about a heap operation, simply an on stack
construction.  And you have such nice looking similar functions which return the converted
value.


But again, do what you want.  I won't mention it again.

Dick