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
_______________________________________________
Tutor maillist - Tutor@python.org
To unsubscribe or change subscription options:
http://mail.python.org/mailman/listinfo/tutor
