[openbiblio-dev] [ckan-dev] coding style
seb.bacon at gmail.com
Wed Jan 26 18:24:16 UTC 2011
On 26 January 2011 17:50, James Gardner <james at 3aims.com> wrote:
> +1 to all this.
> I've never had to resort to @classmethod in my own code - helper
> functions are fine. Agree that self should be changed everywhere it is
> used to cls, it is just misleading at the mo. Also agree there are far
> too many little functions/methods that just do one thing. No need.
> Let's all chip in and fix these as we find them, or does someone want to
> take them on as a single task?
It's too big a task to do in one -- that's mainly what I spent last
week doing on the harvesting code -- about 1.5 days for quite a small
module. Let's just agree to refactor as we go along?
> . On Wed, 2011-01-26 at 18:30 +0100, William Waites wrote:
>> Shall put words to this effect on the coding style wiki page,
>> but wanted to bring it up on the dev lists first (sorry for
>> duplicates, crossposting).
>> Some things that I've noticed in the various codebases that
>> strike me as very ugly:
>> Things like this:
>> def _start_call_timing(self):
>> c.time_call_started = self._get_now_time()
>> def _get_now_time(self):
>> import datetime
>> return datetime.datetime.now()
>> there is no need whatsoever for these methods or ones like
>> this. That's two function calls and five lines of code for
>> what could easily be written as:
>> c.time_call_started = datetime.now()
>> Also, do not import packages, especially standard ones in
>> the body of a function / method unless there's a good reason
>> (like the feature is optional and might throw an import error)
>> Next, there are three problems with the following method:
>> def _uri(self, uri):
>> if isinstance(uri, basestring):
>> return URIRef(uri)
>> return uri
>> first, there is a bug, URIRef inherits from unicode and from
>> basestring so the isinstance check fails -- but this is just
>> a bug, no big deal.
>> second, why is this a method and not a utility function? it
>> has nothing at all to do with the self argument. Functions
>> and methods should use all of their arguments otherwise they
>> belong elsewhere.
>> third, when using @classmethod, the first argument should be
>> cls and not self. self is for instances.
>> That's all for now...
> ckan-dev mailing list
> ckan-dev at lists.okfn.org
More information about the openbiblio-dev