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

[Steven D'Aprano]

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.




-- 
Steven