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

[Dave Angel]

On 11/30/2011 07:49 PM, Steven D'Aprano 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 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.
>
> 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?
>
> (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?
>
> (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.
>
> (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.
>
> (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?
>
> (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.
>
>
I stopped looking at his pastie once the background turned black.  I'd 
have had to copy it elsewhere to even read it.


-- 

DaveA