[Tutor] Best way to construct this

[Alan Gauld]

"Sean Cronin[tutor]" <seancron+pythontutor at gmail.com> wrote

> Right now I have two modules named libndfdsoap and weatherpy. 
> Module
> libndfdsoap contains one function that gets the XML and saves it to 
> a file
> named weather.xml.

Seems fair enough.

> Module weatherpy contains some constants and a class for
> parsing the XML with different functions for find different data 
> types.

The idea of a class is that it hoolds the data plus the functions
that operate *on that data*. Thus it would be more normal to
put your global data into the class itself.

>
> class ByDayXMLParse:

The name is a verb, which does not suggest an Object.
Normally classes are nouns since nouns epresent things.

What kind of thing is this class representing?
It could be an XML Weather document maybe?
In which case I'd expect it to have a parse() method
and the constructor would just open the related file.

>    """Contains functions for parsing the NDFD XML"""
>    def __init__(self, path_to_xml_file):
>    global data, maxtempdata, maxtempnode

Its very unusual to have a lot of global variables in a class.
These could all be internal attributes. That would give the
advantage of allowing you to have multiple instances and
parse them concurrently whereas here the instances
would be sharing data.

>    data = minidom.parse(path_to_xml_file)
>    #Dictionary in which the max temps are going to be stored in the 
> format
>    #{1: day1temp, 2: day2temp, ...}
>    maxtempdata = {}
>    #Less typing for me :)
>    maxtempnode = data.childNodes[0].childNodes[3].childNodes[9
> ].childNodes[1]
>

Personally I'd probably make the parse method separate from
init. I'd just get init to open the file. It could then call parse as
a last line, but by taking parse outside init it gives the option
of reparsing the file later, if for example you suspect the data
has become corrupted somehow.

In pseudo code:

class Weather:
    def init(self, fname):
         self.filename = fname
         self.data = None
         self.maxtemp = None
         self.maxnode = None
         self.parse()

    def parse(self):
         self.data = minidom.parse(self.filename)
         etc as before

    def maxTemp(self):    # no path needed since its stored in the 
object


> I've tried putting the XML parsing code in libndfdsoap but by doing 
> so I
> couldn't access the resulting data.

You could put the functions there and use module cvariables to hold
the result, but more usually you would just return the values and the
importing module (weather in this case would store them)

## in weather code
import libndfdsoap as soap

mylocalvar = soap.myparsingfunc()

However I actually think that would be a mistake since you
are then taking a potentially reusable module which simply
calls a web service and stores the result and putting a lot
of application specific parsing functions into it. I prefer your
approach of building a weather module that reads the file
and keeps the weather specific parsing functions separate
from fetching the data.

>  I can't help but feel like I'm missing
> something that will make my life a whole lot easier.

I think you are on the right track but you probably want to
think a bit more about your class as representing an
object and what kind of things you you want to do to
that object. You can then put the code that instantiates
the object and calls its methods outside in your modules
driver code - and eventually into your Tkinter GUI module.

HTH,

Alan G.