[Tutor] is there a better way to organise this code

[Norman Khine]

hi

On Thu, Dec 1, 2011 at 12:49 AM, Steven D'Aprano <steve at pearwood.info> wrote:
> Norman Khine wrote:
>>
>> hello,
>>
>> is there a better way to organise this code or optimise it.
>> http://pastie.org/2944797
>
>
> Is that a question? Because I get a syntax error in my brain when I parse it
> without the question mark. <wink>

sorry it was suppose to be a question.

>
> Sorry to pick on you, but it astonishes me when people don't bother with
> basic English syntax, and yet try writing code where syntax is *much* more
> important. If they can't be bothered with writing correct English, that
> sends all the wrong signals about the quality of their code.
>
> You should write as if you were coding, otherwise people will assume you
> code like you write.

point taken.

>
> Laziness is one of the cardinal virtues of the programmer, but it has to be
> the right sort of laziness. "Don't reinvent the wheel, use an existing
> library" is good laziness. "Leave out required syntax elements and hope
> someone else will fix them" is not.
>
> Before worrying about optimising the code, how about checking whether it
> works?

i am using the itools library http://www.hforge.org/itools/docs/csv
and the code does work.

>
> (1) What is CSVFile? It appears to be a class, because you inherit from it,
> but it isn't defined anywhere and isn't a builtin. So your code fails on the
> very first line.
>
> (2) You have a class WorldSchema with no methods, and a top-level function
> get_world that *looks* like a method because it has an argument "self", but
> isn't. The indentation is wrong. See what I mean about syntax? Syntax is
> important. So is get_world a wrongly indented method, or a poorly written
> function?
>
> (3) Since get_world doesn't use "self" at all, perhaps it should be a
> top-level function of no arguments? Or perhaps a static method of
> WorldSchema?

this is where i was uncertain as to how best approach it.

>
> (4) You have a class called "getCountries", which seems to be a poor name
> for a class. In general, classes should be *things*, not *actions*. Also I
> recommend that you follow PEP 8 for naming conventions. (Google "PEP 8" if
> you don't know what I mean, and remember, it isn't compulsory, but it is
> recommended.) A better name might be CountryGetter.

ok, will do.

>
> (5) The use of classes appears on first reading to be a Java-ism. In Java,
> everything must be a class Just Because The Powers Who Be Said So. In
> Python, we are allowed, and encouraged, to mix classes and functions. Use
> the right tool for the job. But without any idea of the broader context, I
> have no idea if classes are appropriate or not.

the broader context is that i would like to reuse CountryGetter,
RegionGetter and CountyGetter within a multi-select widget that will
be called from different forms.

>
> (6) getCountries has a method called get_options. Based on the name, a
> reasonable reader would assume it returns some sort of list or dictionary of
> options, right? But according to the documentation, it actually returns a
> JSON "ser", whatever that is. Server? Service? Serialization (of what)?
> Something else?

yes it is a JSON serialization
>
> (7) Other problems:  Enumerate, MSG and iana_root_zone are used but not
> defined anywhere. Documentation is lacking, so I don't understand what the
> code is intended to do. Another class with a poor name, "getRegions". There
> may be others, but I stopped reading around this point.

again sorry about this, i will try to be more explicit next time.

as mentioned above, i was interested in knowing whether or not i
needed to call the get_world() which loads the CSV file every time
each getCountries, getRegion and getCounty classes are called and if
there was a more efficient way to do this?

thanks
>
>
>
>
> --
> Steven
>
> _______________________________________________
> Tutor maillist  -  Tutor at python.org
> To unsubscribe or change subscription options:
> http://mail.python.org/mailman/listinfo/tutor



-- 
%>>> "".join( [ {'*':'@','^':'.'}.get(c,None) or
chr(97+(ord(c)-83)%26) for c in ",adym,*)&uzq^zqf" ] )