FRIHOST FORUMS SEARCH FAQ TOS BLOGS COMPETITIONS
You are invited to Log in or Register a free Frihost Account!


Which approach would rather use?





davidv
So I'm displaying checkboxes and initially this was my code:

Code:
        #self.all = wx.CheckBox(self.panel, label='All types', pos=(840, 104), size=(64, 13), id=1)
        #self.videos = wx.CheckBox(self.panel, label='Videos', pos=(920, 120), size=(48, 16), id=2)
        #self.audio = wx.CheckBox(self.panel, label='Audio', pos=(920, 104), size=(48, 13), id=3)
        #self.images = wx.CheckBox(self.panel, label='Images', pos=(920, 136), size=(56, 13), id=4)
        #self.folders = wx.CheckBox(self.panel, label='Folders', pos=(840, 136), size=(56, 13), id=5)
        #self.docs = wx.CheckBox(self.panel, label='Documents', pos=(840, 120), size=(70, 13), id=6)
        #self.zip = wx.CheckBox(self.panel, label='.zip', pos=(840, 152), size=(56, 13), id=7)
        #self.rar = wx.CheckBox(self.panel, label='.rar', pos=(840, 168), size=(56,13), id=8)
        #self.all.SetValue(True)


That creates 8 different checkbox objects which I can then reference later anywhere within the same class by calling self.x. Then I thought, that's a lot of code repetition. So then I did this:

Code:
def create_option_set(self):
        """ Displays choices the user can pick to process by """
        # Label, position, size, id
        self.all_options_list = [
            ('all', (840, 104), (64, 13), 1),
            ('videos', (920, 120), (48, 16), 2),
            ('audio', (920, 104), (48, 13), 3),
            ('images', (920, 136), (56, 13), 4),
            ('folders', (840, 136), (56, 13), 5),
            ('documents', (840, 120), (70, 13), 6),
            ('.zip', (840, 152), (56, 13), 7),
            ('.rar', (840, 168), (56,13), 8)]
       
        self.options_dict = {}
        self.make_option()
        self.options_dict['all'].SetValue(True)

def make_option(self):
        for checkbox in self.option_list:
            label, pos, size, id = checkbox
            self.option_dict[label] = wx.CheckBox(self.panel, label=label, pos=pos, size=size, id=id)
           


I can then reference each checkbox by calling the dictionary. self.option_dict[x]

The main reason why I changed to the second variation was because I wanted to be able to create an iterable for all my options but it looks overly complicated for something so simple.

Which of the two would you choose and why?
Fire Boar
I'd usually go for the first: it's not code repetition if it's a single function call multiple times with different parameters. The difference here is:

Approach 1: Create checkboxes one at a time, passing parameters directly.
Approach 2: Create list of parameters, one checkbox at a time, then iterate over the list passing them to the checkbox creation function.

I don't really see the need for a list here, it just overcomplicates things.

If you want to have your checkboxes in an iterable, that should be fairly straightforward with the first approach too.
davidv
Fire Boar wrote:
I'd usually go for the first: it's not code repetition if it's a single function call multiple times with different parameters. The difference here is:

Approach 1: Create checkboxes one at a time, passing parameters directly.
Approach 2: Create list of parameters, one checkbox at a time, then iterate over the list passing them to the checkbox creation function.

I don't really see the need for a list here, it just overcomplicates things.

If you want to have your checkboxes in an iterable, that should be fairly straightforward with the first approach too.


I went with the second approach and made some slight changes (I wanted to bind each checkbox to its own event, added more checkboxes and adding one extra item in my tuple). By following the second approach, it halves the amount of code I need to make n checkboxes. Although it does look more complicated than the first approach, it makes adding events much easier in the long run.
jcreus
davidv wrote:
Fire Boar wrote:
I'd usually go for the first: it's not code repetition if it's a single function call multiple times with different parameters. The difference here is:

Approach 1: Create checkboxes one at a time, passing parameters directly.
Approach 2: Create list of parameters, one checkbox at a time, then iterate over the list passing them to the checkbox creation function.

I don't really see the need for a list here, it just overcomplicates things.

If you want to have your checkboxes in an iterable, that should be fairly straightforward with the first approach too.


I went with the second approach and made some slight changes (I wanted to bind each checkbox to its own event, added more checkboxes and adding one extra item in my tuple). By following the second approach, it halves the amount of code I need to make n checkboxes. Although it does look more complicated than the first approach, it makes adding events much easier in the long run.

Definitely I would have done the same.

Repeating code is a mess.

That way, if you want to change anything and, for example, change it to radioboxes or something, you don't have to mess with regular expressions.
Fire Boar
davidv wrote:
I went with the second approach and made some slight changes (I wanted to bind each checkbox to its own event, added more checkboxes and adding one extra item in my tuple). By following the second approach, it halves the amount of code I need to make n checkboxes. Although it does look more complicated than the first approach, it makes adding events much easier in the long run.


Good choice - the second approach is better if you're doing more for each checkbox than just making it. Though in that case, I'd go with a slight adaptation (pseudocode):

Code:
create_checkbox(params_for_first_checkbox)
create_checkbox(params_for_second_checkbox)
create_checkbox(params_for_third_checkbox)
... etc...

function create_checkbox(param1, param2, ...)
  // Code for creating and setting up a checkbox here


Creating an array and immediately looping over it, then discarding that array just doesn't sit right with me.
jcreus
Fire Boar wrote:
davidv wrote:
I went with the second approach and made some slight changes (I wanted to bind each checkbox to its own event, added more checkboxes and adding one extra item in my tuple). By following the second approach, it halves the amount of code I need to make n checkboxes. Although it does look more complicated than the first approach, it makes adding events much easier in the long run.


Good choice - the second approach is better if you're doing more for each checkbox than just making it. Though in that case, I'd go with a slight adaptation (pseudocode):

Code:
create_checkbox(params_for_first_checkbox)
create_checkbox(params_for_second_checkbox)
create_checkbox(params_for_third_checkbox)
... etc...

function create_checkbox(param1, param2, ...)
  // Code for creating and setting up a checkbox here


Creating an array and immediately looping over it, then discarding that array just doesn't sit right with me.

But why that way? It /can/ be useful.

For example, if you are developing a kind of "testing"/"examination" application, it'd be useful to loop over all the checkboxes and know if they are ok. It can be done with the events "on select checkbox", but that way is way simpler and you end up doing the same thing (storing the results in an array), but in this case you have the array done.
Fire Boar
jcreus wrote:
But why that way? It /can/ be useful.

For example, if you are developing a kind of "testing"/"examination" application, it'd be useful to loop over all the checkboxes and know if they are ok. It can be done with the events "on select checkbox", but that way is way simpler and you end up doing the same thing (storing the results in an array), but in this case you have the array done.


I don't really get the problem. If you want to store the checkboxes in an array, that's fine, just create an array containing the checkboxes you create rather than storing each in a separate member variable. The point of contention is the extra array containing the tuples of data required to create the checkboxes in the first place. To me, it seems more sensible to just use a method call, the extra array is unnecessary.
Related topics
Why use IE?
Frankfurt Motor Show, 2005
The Secret Source of Google's Power
Need Advice on a girl (interesting thread)
Decimal, Binary and Hexadecimal
Castlevania returns with an army of monsters and a heart ful
Not Voting is Reasonable for People Who Want Freedom
US forces are apparently THROWING bullets in Iraq now
Rights of the parent ~ rights of the child
8 teens charged in videotaped attack
Dual nature of the Brain?!
I need some tips to have a healthy relationship
About learning on this forum.
ISNT PRACTICAL KNOWLEDGE MISSING SOMEWHERE IN COLLEGES?
'Before' the Big Bang
Reply to topic    Frihost Forum Index -> Scripting -> Others

FRIHOST HOME | FAQ | TOS | ABOUT US | CONTACT US | SITE MAP
© 2005-2011 Frihost, forums powered by phpBB.