On Thu, Mar 6, 2014 at 12:55 AM, Anže Starič <[email protected]> wrote:
> 2014-03-06 9:47 GMT+01:00 <[email protected]>: > > class OneToManyValidator(Validator): > > + """Only tree relationships are allowed. A ticket cannot have > multiple > > + parents.""" > > def validate(self, relation): > > rls = RelationsSystem(self.env) > > - existing_relations = rls._select_relations(relation.source, > > - relation.type) > > + if relation.type != rls.PARENT_RELATION_TYPE: > > + return > > + existing_relations = rls._select_relations(relation.destination, > > + > rls.CHILDREN_RELATION_TYPE) > > I would prefer something like: > > - existing_relations = rls._select_relations(relation.source, > - relation.type) > + existing_relations = rls._select_relations(type=relation.type, > + > destination=relation.destination) > > (and modified select_relations method so it can also filter by > destination). > This way, validator would remain reusable. > That does seem more clear. I wasn't very happy with the inverted logic, but I didn't look at the possibility of changing _select_relations. I won't complain if you refactor my changes ;) In what contexts do you see it being reusable? Do you plan to extract rls.PARENT_RELATION_TYPE to a class variable, or in some way make OneToManyValidator usable as a base class?
