On Wed, Nov 30, 2011 at 8:46 PM, Dave Angel <d...@davea.name> wrote: > 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 > > > There is a button in the upper right corner of the paste bin that lets you switch the hideous black background to something more pleasant. :)
-- Joel Goldstick
_______________________________________________ Tutor maillist - Tutor@python.org To unsubscribe or change subscription options: http://mail.python.org/mailman/listinfo/tutor
