Code review comment for lp:~jimbaker/pyjuju/rel-mgmt-endpoint-join

On Thu, Dec 2, 2010 at 9:42 AM, Jim Baker <email address hidden> wrote:

> On Wed, Dec 1, 2010 at 8:56 AM, Gustavo Niemeyer <email address hidden>wrote:
>
>> Review: Needs Information
>> Thanks Jim. A couple of comments:
>>
>> [1]
>>
>> Given the specific context we're handling, what's the advantage of the
>> approach used when compared to the straightforward option:
>>
>> result = []
>> relation_set1 = yield self.get_relation_endpoints(descriptor1)
>> relation_set2 = yield self.get_rleation_endpoitns(descriptor2)
>> for relation1 in relation_set1:
>> for relation2 in relation_set2:
>> if relation1.may_relate_to(relation2):
>> result.append((relation1, relation2))
>> return result
>>
>>
> Both implement the nested loops join. The advantage of the recipe I wrote
> is that it's designed for composability. Think about the idea of linq. At
> this time, that's not in the picture, and usually isn't until we have more
> such queries, or their variants (only care if at least one result exists,
> for example).
>
> So it probably makes more sense to just write it as you describe.
>

In the latest version of the branch I pushed.

>
>
>>
>> [2]
>>
>> Individual tests which verify a specific intended aspect, including a nice
>> docstring which explain its intended outcome, and perhaps some additional
>> detail
>> about why it might be wrong (in some situations), are generally more
>> understandable and easier to maintain in the future.
>>
>
> Agreed, I've changed to adopt this practice in my latest branch on batch
> topology changes.
>
>

I have split these tests in the latest version of the branch.