Geometry Boost List: [ggl] combine

Subject: [ggl] combine
From: Adam Wulkiewicz (adam.wulkiewicz)
Date: 2011-02-27 13:00:01

Barend Gehrels wrote:
> Hi Adam,
>
>
>> IMO code should be self describing.
>
> Agreed.
>
>> What I was trying to say is that if the function's name is a word
>> refering to the parameter, it should be the first one,
>
> OK, I understand. So box is expanded so the first parameter. Sounds
> reasonable.
>
> But what about intersection?
> intersection(red, blue, output);
>
> here the result, the "intersection", is the last one. (Among other
> things) to confirm with the inserter,
> intersection_inserter(red, blue, std::back_inserter(output));
>
>
>> so:
>>
>> expand(this_object, by_something_else)
>> void expand(Box&, Geometry const&)
>>
>> or
>>
>> expanded_object = expanded(this_object, by_something_else)
>> Box expanded(Box const&, Geometry const&)
>>
>> If the order of parameters shouldn't be straightforward, function
>> should have the name refering to both parameters so combine is
>> probably better name, but then you don't know what this function
>> exactly does. It should be the name describing that both parameters
>> are used to create a box.
>>
>> I don't have a good name but it might be something like
>>
>> box = boxify(box, geometry)
>> box = boxify(geometry, box)
>> box = boxify(geometry1, geometry2) ?
>>
>> Or, since there is make_envelope already
>>
>> box = make_envelope<Box>(geometry)
>> box = make_envelope(box, geometry)
>> box = make_envelope(geometry, box)
>> box = make_envelope<Box>(geometries.begin(), geometries.end()) ?
>> box = make_envelope<Box>(geometries_range) ?
>>
>> Personally, I'd like to have simply
>>
>> void expand(Box&, Geometry const&)
>
> I like this too. It is how it was (just renamed from combine). I agree
> that expand is a better name, it better indicates what happens.
>
> So do we have a reason to say, "here it is OK to have this mutable
> parameter first, but most other functions have them as last".
>
> Maybe because of your earlier example. If C++ would allow extension
> methods (would be nice), as C# does, its signature could be written as
> such:
> void expand(this Box&, Geometry const&);
>
> A user could then call
>
> box.expand(Geometry),
>
> even if expand is not a method of his box. So then it MUST be the first
> parameter.
>
> For intersection this does not apply, because the intersection is always
> a new thing... this is not intuitive:
>
> result.intersection(red, blue);
>
> So maybe that is a good reasoning.

Yes, the intersection isn't mutable. It's a new object. According to my 
logic it should be returned by the function.

output = intersection(red, blue);

Output iterator should be the last parameter because that is how it is 
implemented and it shouldn't be changed.

intersection_inserter(red, blue, std::back_inserter(output));

And mutable object should be the first parameter.

add_to_intersection(inters, red); // just an example

So, in the case of boxes there could be:

new_box = make_envelope(box, geometry)
new_box = expansion(box, geometry)

boxes_insert(
   geometries_first, geometries_last, boxes_output_iter)

expand(mut_box, geometry)
extend(mut_box, geometry)

Btw, to be completly honest, intersection_inserter should be the name of 
an object, not function. Function's name should be the verb:

intersection_insert(red, blue, std::back_inserter(output));
// or some similar name e.g. intersection_copy

or e.g.

intersect(
   geometries.begin(),
   geometries.end(),
   std::back_inserter(output)
); // push back the intersection of all geometries in range

but (for inserter)

// just an example

std::vector<std::pair<Geometry1, Geometry2> > g_pairs;
std::vector<Intersection> g_intersections;
/*...*/
std::copy(
   g_pairs.begin(),
   g_pairs.end(),
   geometry::intersection_back_inserter(g_intersections)
);
// push back intersections of pairs of geometries

What do you think?

Regards,
Adam