[prev in list] [next in list] [prev in thread] [next in thread] 

List:       vtk-developers
Subject:    Re: [vtk-developers] [Paraview] vtkEllipsoidSource class
From:       David Doria <daviddoria+vtk () gmail ! com>
Date:       2009-10-15 20:16:10
Message-ID: c19fcadc0910151316l25bc1a43sc9c819f86f9276b7 () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


>    David,
>>>
>>>    I see a lot of calls to X::New() but no calls to X->Delete().
>>>    You've got to be more careful about memory leaks. Some of the
>>>    examples you posted on the wiki also have leaks. There is a flag
>>>    VTK_DEBUG_LEAKS set it to on and you will get a message about any
>>>    leaks you create.
>>>
>>>    By the way, in addition before I show my code to anyone I want to
>>>    run valgrind --leak-check=full --show-reachable=yes to verify that
>>>    I haven't overlooked anything. Often this catches serious errors.
>>>
>>>    You may consider using shallow copy rather than deep copy when
>>>    possible it's faster and uses less memory.
>>>
>>>    You have tabs in the source, you should use 2 spaces to conform to
>>>    KW style (your editor should handle for you).
>>>
>>>    Hope this helps,
>>>    Burlen
>>>
>>>
As per Burlen's suggestions, I fixed all of the memory leaks and got rid of
the deep copies.  I also made vtkEllipsoidSource the superclass of
vtkSphereSource, as this makes much more sense, as a sphere IS AN ellipsoid
and hence the inheritance is sound.

The updated code is in the same location as before:
http://www.rpi.edu/~doriad/VTK_List/vtkEllipsoidSource/

I named the new sphere classes vtkSphereSource2 for clarity for the moment.

I tried to conform to the VTK style of indentation and brackets - but I know
I left some tabs in there still... is there an automated way to change these
tabs to the 2 space style? My editor (KDevelop) lets me set what happens to
FUTURE tab keypresses, but I didn't see anything that would go through and
change all existing tabs to the new setting. Suggestions?

Thanks,

David

[Attachment #5 (text/html)]

<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px \
solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div><div \
class="h5"><div class="gmail_quote"><blockquote class="gmail_quote" \
style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; \
padding-left: 1ex;"> <blockquote class="gmail_quote" style="border-left: 1px solid \
rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div><div>  \
David,<br> <br>
    I see a lot of calls to X::New() but no calls to X-&gt;Delete().<br>
    You&#39;ve got to be more careful about memory leaks. Some of the<br>
    examples you posted on the wiki also have leaks. There is a flag<br>
    VTK_DEBUG_LEAKS set it to on and you will get a message about any<br>
    leaks you create.<br>
<br>
    By the way, in addition before I show my code to anyone I want to<br>
    run valgrind --leak-check=full --show-reachable=yes to verify that<br>
    I haven&#39;t overlooked anything. Often this catches serious errors.<br>
<br>
    You may consider using shallow copy rather than deep copy when<br>
    possible it&#39;s faster and uses less memory.<br>
<br>
    You have tabs in the source, you should use 2 spaces to conform to<br>
    KW style (your editor should handle for you).<br>
<br>
    Hope this helps,<br>
    Burlen<br><br></div></div></blockquote></blockquote></div></div></div></blockquote><div><br>As \
per Burlen&#39;s suggestions, I fixed all of the memory leaks and got rid of the deep \
copies.  I also made vtkEllipsoidSource the superclass of vtkSphereSource, as this \
makes much more sense, as a sphere IS AN ellipsoid and hence the inheritance is \
sound.<br> <br>The updated code is in the same location as before:<br><a \
href="http://www.rpi.edu/~doriad/VTK_List/vtkEllipsoidSource/">http://www.rpi.edu/~doriad/VTK_List/vtkEllipsoidSource/</a><br><br>I \
named the new sphere classes vtkSphereSource2 for clarity for the moment.<br> <br>I \
tried to conform to the VTK style of indentation and brackets - but I know I left \
some tabs in there still... is there an automated way to change these tabs to the 2 \
space style? My editor (KDevelop) lets me set what happens to FUTURE tab keypresses, \
but I didn&#39;t see anything that would go through and change all existing tabs to \
the new setting. Suggestions?<br> <br>Thanks,<br><br>David <br></div></div>



_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://www.vtk.org/mailman/listinfo/vtk-developers



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic