On Fri, Oct 02, 2009 at 10:44:10AM -1000, David Cantrell wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On Tue, 29 Sep 2009, Joel Granados wrote:
>
>> On Fri, Sep 25, 2009 at 01:55:10PM -1000, David Cantrell wrote:
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> Comments below.
>>>
>>> On Wed, 16 Sep 2009, Joel Granados Moreno wrote:
>>>
>>>> * iw/partition_gui.py (treeSelectCB): Make sure that the graph is being
>>>> painted by the right StripeGraph class. We decide which graph to use
>>>> based on the device we receive.
>>>> (resetCB, refresh, viewButtonCB, getScreen, populate): Change the custom
>> ....
>>>> + self.stripeGraph.selectSliceFromObj(device)
>>>> +
>>>> + elif isinstance(device, storage.LVMVolumeGroupDevice):
>>>> + if not isinstance(self.stripeGraph, LVMStripeGraph):
>>>> + self.stripeGraph.shutDown()
>>>> + self.stripeGraph = LVMStripeGraph(self.tree, self.editCB, self.storage, device)
>>>> + self.stripeGraph.setDisplayed(device)
>>>> +
>>>> + elif isinstance(device, storage.LVMLogicalVolumeDevice):
>>>> + if not isinstance(self.stripeGraph, LVMStripeGraph):
>>>> + self.stripeGraph.shutDown()
>>>> + self.stripeGraph = LVMStripeGraph(self.tree, self.editCB, self.storage, device.vg)
>>>> + self.stripeGraph.setDisplayed(device.vg)
>>>> + self.stripeGraph.selectSliceFromObj(device)
>>>> +
>>>> + elif isinstance(device, storage.MDRaidArrayDevice):
>>>> + if not isinstance(self.stripeGraph, MDRaidArrayStripeGraph):
>>>> + self.stripeGraph.shutDown()
>>>> + self.stripeGraph = MDRaidArrayStripeGraph(self.tree, self.editCB, self.storage, device)
>>>> + self.stripeGraph.setDisplayed(device)
>>>
>>> Is there a way to accomplish this stuff without using isinstance()?
>>
>> I could use the __class__ of the object. Any thing against isinstance?
>
> This sort of code strikes me as not very object oriented. Why not have a
> method on each of the *Device classes that takes in self.stripeGraph and then
This is how its coded. If you look at each if, at the end there is a
self.stripeGraph.setDisplayed(device) . This function is exactly what
you propose. Additionally there is
self.stripeGraph.selectSliceFromObj(device), that also does its stuff
according to device.
I'm doing 2 things with is instance:
1. Getting the type of device that is selected in the tree view.
This is necessary to choose the correct object for the self.strpeGraph
object.
2. Getting the current self.stripeGraph class. If the current class is
different from what I need, I instantiate a new object. If the class is
the same, I don't instantiate a new graph object but set the device to
be displayed with the existing object.
I could create an intermediate class that handles all this. This class
would have a list of possible graph objects to use and would not
instantiate new ones every time it is needed. Aside from that, the code
would pretty much be the same. I'll work on it today and post a patch.
THX for the review, much appreciated.
Regards.
> does what it needs to do specific to that class? Could be reduced to:
>
> device.display(self.stripeGraph, ...)
>
> Where ... is the other stuff necessary such as self.tree, self.editCB,
> self.storage, and so on.
>
> I just think that LVMVolumeGroupDevice, LVMLogicalVolumeDevice, and
> MDRaidArrayDevice should know how to display themselves given the right
> information.
>
>>>> +
>>>> + else:
>>>> + # Don't really want to do anything in this case.
>>>> + return
>>>>
>>>> def deleteCB(self, widget):
>>>> """ Right now we can say that if the device is partitioned we
>>>> @@ -1367,15 +1399,15 @@ class PartitionWindow(InstallWindow):
>>>> def resetCB(self, *args):
>>>> if not confirmResetPartitionState(self.intf):
>>>> return
>>>> -
>>>> - self.diskStripeGraph.shutDown()
>>>> +
>>>> + self.stripeGraph.shutDown()
>>>> self.storage.reset()
>>>> self.tree.clear()
>>>> self.populate()
>>>>
>>>> def refresh(self, justRedraw=None):
>>>> log.debug("refresh: justRedraw=%s" % justRedraw)
>>>> - self.diskStripeGraph.shutDown()
>>>> + self.stripeGraph.shutDown()
>>>> self.tree.clear()
>>>>
>>>> if justRedraw:
>>>> @@ -1575,10 +1607,10 @@ class PartitionWindow(InstallWindow):
>>>> vgeditor.destroy()
>>>>
>>>> def viewButtonCB(self, widget):
>>>> - self.show_uneditable = not widget.get_active()
>>>> - self.diskStripeGraph.shutDown()
>>>> - self.tree.clear()
>>>> - self.populate()
>>>> + self.show_uneditable = not widget.get_active()
>>>> + self.stripeGraph.shutDown()
>>>> + self.tree.clear()
>>>> + self.populate()
>>>>
>>>> def getScreen(self, anaconda):
>>>> self.anaconda = anaconda
>>>> @@ -1623,8 +1655,7 @@ class PartitionWindow(InstallWindow):
>>>> self.treeView.connect('row-activated', self.treeActivateCB)
>>>> self.treeViewSelection = self.treeView.get_selection()
>>>> self.treeViewSelection.connect("changed", self.treeSelectCB)
>>>> - self.diskStripeGraph = DiskStripeGraph(self.tree, self.editCB,
>>>> - storage = self.storage)
>>>> + self.stripeGraph = StripeGraph(self.tree, self.editCB)
>>>> self.populate(initial = 1)
>>>>
>>>> # Create the top scroll window
>>>> @@ -1632,7 +1663,7 @@ class PartitionWindow(InstallWindow):
>>>> hadj = gtk.Adjustment(step_incr = 5.0)
>>>> vadj = gtk.Adjustment(step_incr = 5.0)
>>>> swt = gtk.ScrolledWindow(hadjustment = hadj, vadjustment = vadj)
>>>> - swt.add(self.diskStripeGraph.getCanvas())
>>>> + swt.add(self.stripeGraph.getCanvas())
>>>> swt.set_policy(gtk.POLICY_AUTOMATIC, gtk.POLICY_AUTOMATIC)
>>>> swt.set_shadow_type(gtk.SHADOW_IN)
>>>>
>>>>
>>>
>>> The rest looks fine.
>>>
>>> - -- David Cantrell <dcantrell@xxxxxxxxxx>
>>> Red Hat / Honolulu, HI
>>>
>>> -----BEGIN PGP SIGNATURE-----
>>> Version: GnuPG v1.4.9 (GNU/Linux)
>>>
>>> iEYEARECAAYFAkq9WF4ACgkQ5hsjjIy1VkmurwCglDcx+3pqOH4HuXxU3HnZaqEL
>>> cs8AoMzwwXq/4AYHHfjFsE59k/GuwXPs
>>> =UvnX
>>> -----END PGP SIGNATURE-----
>>>
>>> _______________________________________________
>>> Anaconda-devel-list mailing list
>>> Anaconda-devel-list@xxxxxxxxxx
>>> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
>>
>>
>
> - -- David Cantrell <dcantrell@xxxxxxxxxx>
> Red Hat / Honolulu, HI
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkrGZhsACgkQ5hsjjIy1VkkMjQCfU0rn7o5eIcnBOHHHUTEcFqxO
> jJIAnRLmgfoPRr5uMPq8v1dRyXeu6xpO
> =wY0l
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
--
Joel Andres Granados
Brno, Czech Republic, Red Hat.
Attachment:
pgpLcfbMKHHDF.pgp
Description: PGP signature
_______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
