On 07/12/2010 06:29 PM, Sebastian Benthall wrote:
These shortcut methods will error if, for whatever reason a layer, has
more than one poc or metadata_author. It doesn't seem unreasonable to
expect multiple of either, so if we are going this way then we should
probably ditch the properties and make developers deal with the idea
that there might be more than one party playing any particular
role for
any particular layer. Otherwise, we should add a uniqueness
constraint
on ContactRole on (layer, role).
We had a similar discussion, if I recall correctly, about the use of
shortcut property methods on the Layer model to access information
from the Catalog. If this is something that is going to keep coming
up, it would be good to talk about it and arrive at a consensus.
So, I'm going to ramble on it for a while.
Personally, I'm in favor of shortcuts as long as they don't error. I
don't think "making the developer deal with the idea" in software
design is a much more productive attitude than "make the user deal
with the idea" in user interface design.
I certainly can agree to disagree on this. And David, if you think
its very important and especially if you see a technical reason for
it, I wouldn't be opposed to some kind of coding policy around it.
But from my vantage point, it seems on one level like an
unsubstantive issue of coding style.
But I can think of one practical reason to encourage these kinds of
shortcuts. it seems like the heavy-model/light-view coding pattern
and the use of @property decorations as a shortcut for field access
are common expectations for Python/Django developers (somebody who
knows better what they are talking about should feel free to correct
me). I wouldn't want our code to alienate potential contributors who
are attracted to the project because they are good at customizing
Django apps.
So I am in favor of not restricting this sort of thing, at least until
we can see tangibly where it becomes a problem (we can always change
it then).
If, on the other hand, we decide to take a position against property
decorations on the Models, it would be good to have a policy about it
that can be communicated in our coding standards rather than just
strongly encouraged during code reviews. We should then refactor the
existing code to be in line with those standards. Otherwise, I think
this is going to be a source of confusion and possibly frustration for
contributors moving forward.
--
Sebastian Benthall
OpenGeo - http://opengeo.org
A few points:
1) The property in question is not just unnecessary, it is incorrect.
The incorrectness is a much bigger deal (to me) than the property-ness.
2) Good API design, like good UI design, is all about choosing the
proper level of abstraction (and good naming/labelling, and all of that.
but the abstraction is key.) The proper level of abstract is not always
the one that hides the most information. It is ridiculous to write a
model method for every possible query on related objects (foreign
keys/many-to-many relations) we might want to do, when we have a nice
simple ORM method for querying by arbitrary criteria.
2b) It is much tougher to remove methods from an API that is in use than
to add them.
3) The official Python style guide
(http://www.python.org/dev/peps/pep-0008/) provides these guidelines
regarding the use of properties:
Note 1: Properties only work on new-style classes.
Note 2: Try to keep the functional behavior side-effect free, although
side-effects such as caching are generally fine.
Note 3: Avoid using properties for computationally expensive
operations; the attribute notation makes the caller believe
that access is (relatively) cheap.
I don't think that we need to address properties beyond this in our own
style guidelines. However, Note 3 (debatably) applies to all of the
GeoServer and GeoNetwork access properties.
4) One technical argument against using annotations in general is that
annotation syntax a new Python feature in version 2.6. @property does
not work in some alternative Python implementations, notably Jython.
This is not an argument against properties themselves since decorators
are pure syntactic sugar.
