[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->Delete().<br>
You'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't overlooked anything. Often this catches serious errors.<br>
<br>
You may consider using shallow copy rather than deep copy when<br>
possible it'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'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'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