Check my refactor for efficiency please.

wxPython Users
1

Chris you're right, I could call the buttons 0, 1, 2, but it's how I started
out, before learning better techniques, and that lists start with 0.

As for what the buttons are for...

I'm writting a program that will open a log file (of the users choosing,)
and read the last entry in that log file, and pubsub it to another frame.
For example:

I want to see the log entry that contains "green hats" to go to the
frame "Green hat;" and the log entry that contains "red socks" to
go to the frame "Red socks." In real time.

So the log is continually read for the latest log entry.

Here I use container as a name for the wx.Frame with a textctrl in it
The point of the buttons, and objects in this part of the program I'm
working on is for the user to define, the label for the contianer,
the search string to be used to sort log entries that go into the container
the foreground, and background color, font, and optionally a wav fule
to note that the occurance of "X" has been publushed to it's respective
frame.

I had a few issues, and the gang here has been fantastic in helping,
teaching, mentoring, scrutinizing my code. And from time to time
knocking me around a wee-bit when my code it outright wrong.

Originally nothing was refactored. No patterns, no nothing. Every
object was coded, every bind, event handler etc... 12 buttons, 12
binds, 12 event handlers. Now I'm working with 12 buttons and scads
of other widgets more efficiently coded, and the 12 buttons, with
2 binds, and 2 event handlers to deal with the buttons.

Ultimately, I had a py file that worked..... ugly and apparantly hard to
read. I could read it today, but in the future that would not be so true.
because I'd have forgotten what the quirks were. And an EXE that
didn't work.

So I thought if I made the folks here happy that would reflect what what
py2exe was looking for to work right and be happy, and it would work
as an exe too.

Steve

···

----- Original Message ----- From: "Christopher Barker" <Chris.Barker@noaa.gov>
To: <wxpython-users@lists.wxwidgets.org>
Sent: Wednesday, 23 July, 2008 13:04
Subject: Re: [wxpython-users] Check my refactor for efficiency please.

Again, efficiency is not the point here!

    def CDefSnd(self, event):
        self.btn = event.GetEventObject()
        for x in range(0,5):
                if self.btn == self.Containers['SndBtn']:
                    print x # dialog will go here (this is simpler)
        event.Skip()
The bottom code works, I know that button 1, is list index 0
and button 6, is list index 5... it works.

Why not call them button0, button1, etc?

But anyway, presumably each of those buttons is related to something, hopefully some object in your system. So what you really want wot know is which object the button is supposed to do some action on. In this case, perhaps you have a list of those objects, parallel to the Containers list, so you can index into that to get the object.

-Chris

--
Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov
_______________________________________________
wxpython-users mailing list
wxpython-users@lists.wxwidgets.org
http://lists.wxwidgets.org/mailman/listinfo/wxpython-users

2

Steve,

The exe not working was the catalyst for this anyhow. If it had worked, I probably
wouldn't have cared if it was 1300 lines of garbage... But what I'm hoping is
that when the code is "better looking" py2exe will think so as well.

It's funny to me that the py file works from IDLE, but the exe doesn't. Now
others have had issues with my code, so I wonder if my PC is magic.

py2exe can take a bit of tweaking to get it right. It should generate an error log that you could post and we could look at. When I first created mine, it ran fine on about 99% of the PCs I tried it on and mysteriously failed on one or two. In my case, I needed an extra dll or two. Hopefully your issue is something simple too.

When you're ready again, post your code and we'll give her a go.

···

-------------------
Mike Driscoll

Blog: http://blog.pythonlibrary.org
Python Extension Building Network: http://www.pythonlibrary.org

3

ah!, you really should have posted this earlier -- time for another re-factor!

Well I've been thinking about the user being able to define how many
frames they want, and I've determined on a given monitor there is only
so much usable realestate to show X number of frames.

The idea was that people with 2+ monitors attached to a machine
could use the program to take "chat boxes" out of the game, and
re-locate those boxes on another monitor, thus you have a need
to read a log file generated by this game.

Maybe, but the real strength of the refactoring you've been doing is that it opens the door to making it arbitrary -- it wouldn't be hard at all!

Right, but going back to the above, another issue could be pure processing
power of the log entry grabber. I feel that the more frames there are, I might
actually need to start threading the get and sort functions.

First, some suggestions on a GUI re-factor. You've got four things you want set: two colors, a font and a sound -- why have those be buttons? why not text controls, or choice controls, or.... you shouldn't have to click a button and get a dialog to set one value.

You're right. Might be a better way to go... But wxPython already has
prebuild dialogs... that really make some facets of GUI building simple.

wx.lib.filebrowsebutton is easy to build.
wx.lib.colourselect is neat
and the ease of configuring the font dialog (disabling effects)
even a not related dialog wx.AboutDialogInfo() is cool

these are already in wxPython, so I don't particularly want to build my
own custom widgets if I don't have to. Tell ya what... If I get this
program done, refactored, function and working properly as an EXE,
I will probably do just that... first things first.

I'd still do it this way -- have a dialog pop up to set the values for one Container, rather than show all six at once, but whatever works for you...

You right still. Probably a better way to go... You are right... could very
easily say at the top "How many contianers do you want." and that number
now becomes the 'b' in:

dictlist =
b = user_input + 1
"for x in range (a,b):
    dict = {}
    code for the button and sizers for the button here
    then append each dict to dictlist...

and viola, have 1000 containers if you saw fit.

Part of my reluctance to this is I'm trying to draw a line in the sand,
because I'm beginning to think I'm trying to "build a better mouse trap."
You know what I mean? Oh, this would be better if I did X....
and I really should add Y here... and change Z this way because
that example on www.bettercoderthanyouare.com has a really
cool "object" I just have to add to my GUI.

I know need a dictionary to hold the data.

optionsdict = {'C1BGColor':self.C1BGColorDlg.GetValue()}
that sort of thing...

That's a good way to do it, but why specify C1 (or C2, or C?) hard coded. It looks like you have a label for each, so use that as the key:

What I'd do is have something like:

optionsdict = {"label1": {"BGColor": "Red",
                          "FGColor": "Black",
                           .....
                          },
               "label2": {"BGColor": "Blue",
                          "FGColor": "White",
                           .....
                          },
                ...
               }

Now you have a dict that holds a bunch of dicts, each with a set of data for one Container. Easy to extend to N containers. Better yet, you might want to use a class to represent a container:

class Container:
    def __init__(self, BGColor="White", FGColor="Black",....)
        self.BGColor = BGColor
        self.FGColor = FGColor
        ....

Then you can put an instance of that class for each Container in your OptionsDict.

I do the same thing, except I use a list instead of a dictionary... almost the same.
you call yours with a key, I call mine with an index. I don't know if the advantage
of one versus another is substantial... (Wow, I almost sounded like I knew what
I was talking about there.)

Now the GUI Code:

What you have is six (really N) collections of controls all the same. Each collection represents one container. So, what you want is to create a class for that container, rather than one for all of them. You ant to subclass wx.Panel, and put the controls on it.

Now, to build your whole Window, you put N instances of that wx.Panel subclass on the main window.

This also would make it easy to re-factor, and put a list of the Container in the mins window, and bring up your settings Panel in a dialog when the user wants to edit the settings.

in MVC terms, the Container class is your model, the setting panel is your view/controller.

Right, the way it is right now it could have "N" instances. I've played with
the code a bit. I'm sure it's possible to say:

<PSUEDO CODE>
if N instance > 6 and < 12:
    put first instances in this box sizers (wx.VERTICAL)
    put instances after 6 in box sizer 2 (wx.VERTICAL)
    stick those two box sizers in box sizer 3 (wx.HORIZONTAL)
</PSUEODO CODE>

and wouldn't have to change the layout of the panel (for a

Does that make any sense?

Yes, perfect sense! and I've gleaned some more insight too.
But I think for the time being I'll still stick with 6 instances
to try and keep the mouse trap small, and draw a line in the
sand with regard to features.

···

----- Original Message ----- From: "Christopher Barker" <Chris.Barker@noaa.gov>
To: <wxpython-users@lists.wxwidgets.org>

-Chris

--
Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov
_______________________________________________
wxpython-users mailing list
wxpython-users@lists.wxwidgets.org
http://lists.wxwidgets.org/mailman/listinfo/wxpython-users

4

Steve Freedenburg wrote:

Well I've been thinking about the user being able to define how many
frames they want, and I've determined on a given monitor there is only
so much usable realestate to show X number of frames.

OK, so we'll leave that aside for now.

Actually I started playing with thecode to do what you are talking about...
I'm such a push over.... The desire to see how it's done outweighed
the desire to complete the project. Same mission, diferent way of
going about it.

You're right. Might be a better way to go... But wxPython already has
prebuild dialogs... that really make some facets of GUI building simple.

fair enough.

Part of my reluctance to this is I'm trying to draw a line in the sand,
because I'm beginning to think I'm trying to "build a better mouse trap."
You know what I mean?

Absolutely. as they say 'sometimes "better" is the enemy of good "enough" '

However, you seem to be treating this as as much of a learning opportunity as a just trying to get a job done, so...

What I'd do is have something like:

optionsdict = {"label1": {"BGColor": "Red",
                          "FGColor": "Black",
                           .....
                          },

I do the same thing, except I use a list instead of a dictionary... almost the same.

yup -- a list is fine. and it keeps order, which you may want.

Now the GUI Code:

What you have is six (really N) collections of controls all the same. Each collection represents one container. So, what you want is to create a class for that container, rather than one for all of them. You ant to subclass wx.Panel, and put the controls on it.

Right, the way it is right now it could have "N" instances. I've played with
the code a bit. I'm sure it's possible to say:

<PSUEDO CODE>
if N instance > 6 and < 12:
   put first instances in this box sizers (wx.VERTICAL)
   put instances after 6 in box sizer 2 (wx.VERTICAL)
   stick those two box sizers in box sizer 3 (wx.HORIZONTAL)
</PSUEODO CODE>

and wouldn't have to change the layout of the panel (for a

I think you've missed a key point I was trying to make. They key is that the way you've got it now is that you are building one panel with 36 controls, six of which correspond to each container, but they are all 36 all on one Panel. Automating the building of these makes it doable, but it would be cleaner to think of it differently:

Build a panel with six "controls", one for each container.. each of those controls is a wx.Panel subclass (Call it ContainerPrefsPanel) that holds the 6 controls you need, plus all the logic for getting and setting the attributes of your Container class. Each instance of ContainerPrefsPanel is associated with a single Container object (it will probably be an attribute of the Panel). That way you don't need to have similar names like:

I am putting all the controls for a given frame in one panel. That screen
I showed you actually consisted of 8 panels. The top panel for the
log directory and character slice, and 6 more panels created through the
for x in range(1,7) build xyz... all the widgets, and sizers are on a
panel, and that panel is placed X times. I have already pulled that code
out of my program and stuck it in it's own frame, now it's just a matter of
asking the user "How many containers do you want?" they say "X"
and "X" buttons pop up, Container 0 through Container 10 or whatever.

So as far as utilizing / reusing code, it was a simple pull form here, and
insert there, with little modification other than what's need to make data
from one class be available in another class.

···

--
Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov
_______________________________________________
wxpython-users mailing list
wxpython-users@lists.wxwidgets.org
http://lists.wxwidgets.org/mailman/listinfo/wxpython-users

5

Steve Freedenburg wrote:

The desire to see how it's done outweighed
the desire to complete the project.

sorry about that :wink:

I am putting all the controls for a given frame in one panel. That screen
I showed you actually consisted of 8 panels. The top panel for the
log directory and character slice, and 6 more panels created through the
for x in range(1,7) build xyz... all the widgets, and sizers are on a
panel, and that panel is placed X times.

OK -- I'll try one more time:

Not just different panels, but each panel being an instance of a custom subclass of wx.Panel -- that's the key, and I don't think it's what you are doing:

Read this:

http://wiki.wxpython.org/wxPython%20Style%20Guide

particularly #7

example pseudo-code:

class ContainerPrefPanel(wx.Panel):
     def __init___(self, Prefs, *args, **kwargs):
         wx.Panel.__init__(self, *args, **kwargs)

  self.Prefs = Prefs

  self.BGButton = wx.Button(self, label=BAckground Color)
         ....
         all your button creation, sizer and event binding code here.
         ...

     def OnBGButton(self, evt):
          # bring up colordialog
          ....
          self.Pref.BGColor = color
     #similarly for the other controls....

now your Frame Class:

class ContainerFrame(wx.Frame):
     def __init__(self, ....):

     .....
     for container in containers:
         P = ContainerPrefPanel(container, self)
         ...sizer code...

     The other buttons, etc....

Does that help any?

-Chris

···

--
Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov

6

Yes I see what you mean, no it's not in it's own class, it was just code
stuck in the class
for the config frame. I'm playing with it now. I'm trying to put that code
in it's own
class, and the user say I want "x" windows, and "x" buttons pop up in the
frame, one button
per configurable contianer. I think you meantioned something about that
earlier...

···

----- Original Message ----- From: "Christopher Barker" <Chris.Barker@noaa.gov>
To: <wxpython-users@lists.wxwidgets.org>
Sent: Thursday, 24 July, 2008 18:32
Subject: Re: [wxpython-users] Check my refactor for efficiency please.

Steve Freedenburg wrote:

The desire to see how it's done outweighed
the desire to complete the project.

sorry about that :wink:

I am putting all the controls for a given frame in one panel. That
screen
I showed you actually consisted of 8 panels. The top panel for the
log directory and character slice, and 6 more panels created through the
for x in range(1,7) build xyz... all the widgets, and sizers are on a
panel, and that panel is placed X times.

OK -- I'll try one more time:

Not just different panels, but each panel being an instance of a custom
subclass of wx.Panel -- that's the key, and I don't think it's what you
are doing:

Read this:

wxPython Style Guide - wxPyWiki

particularly #7

example pseudo-code:

class ContainerPrefPanel(wx.Panel):
    def __init___(self, Prefs, *args, **kwargs):
        wx.Panel.__init__(self, *args, **kwargs)

self.Prefs = Prefs

self.BGButton = wx.Button(self, label=BAckground Color)
        ....
        all your button creation, sizer and event binding code here.
        ...

    def OnBGButton(self, evt):
         # bring up colordialog
         ....
         self.Pref.BGColor = color
    #similarly for the other controls....

now your Frame Class:

class ContainerFrame(wx.Frame):
    def __init__(self, ....):

    .....
    for container in containers:
        P = ContainerPrefPanel(container, self)
        ...sizer code...

    The other buttons, etc....

Does that help any?

-Chris

--
Christopher Barker, Ph.D.
Oceanographer

Emergency Response Division
NOAA/NOS/OR&R (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception

Chris.Barker@noaa.gov
_______________________________________________
wxpython-users mailing list
wxpython-users@lists.wxwidgets.org
http://lists.wxwidgets.org/mailman/listinfo/wxpython-users

7

Steve Freedenburg wrote:

Yes I see what you mean, no it's not in it's own class, it was just code
stuck in the class
for the config frame. I'm playing with it now. I'm trying to put that code
in it's own
class, and the user say I want "x" windows, and "x" buttons pop up in the
frame, one button
per configurable contianer. I think you meantioned something about that
earlier...

I think so -- let us know what you come up with.

-Chris

···

--
Christopher Barker, Ph.D.
Oceanographer

NOAA/OR&R/HAZMAT (206) 526-6959 voice
7600 Sand Point Way NE (206) 526-6329 fax
Seattle, WA 98115 (206) 526-6317 main reception