Boost logo

Geometry :

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)

   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.

); // 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;
// push back intersections of pairs of geometries

What do you think?


Geometry list run by mateusz at