[OSM-dev] Osmium: more tests and some questions

[Johannes Kolb]

Hi Jochen, hi OSM developers,

in the last days I've written some tests for the objects defined in
osmium/osm/*.hpp.
Again, I will send a pull request with this changes.

I also want to share some remarks/proposals with you:

osmium/osm/object.hpp(235):
  Is there a reason, why set_attribute() can't set the endtime attribute?

osmium/osm/node.hpp(98):
    I wonder if it is a good idea to overload the comparison of
shared_ptrs. At
    the moment you have to be very careful, if you want to compare
shared_ptrs to
    Nodes, since the comparsion is only overloaded for shared_ptr<Node
const>.
    If you have shared_ptr<Node> for example, they are compared by the
default
    comparison (which is probably by memory location).

    I would recommend to remove the comparison operator for shared_ptrs. You
    can compare shared_ptrs by dereferencing them (*lhs < *rhs) anyway.

osmium/osm/way_node.hpp(82):
    For WayNode objects only the comparison lhs < rhs works,
    lhs > rhs gives a compiler error.
    Perhaps it is necessary to implement operator> too?

osmium/osm/way.hpp(77):
    In the comment for add_note you write
    "Will throw a range error if the way already has max_nodes_in_way
nodes."
    This is nowhere implemented; not even is max_nodes_in_way defined
somewhere.

osmium/osm/way_node_list.hpp(111):
    According to the documentation at
    http://www.cplusplus.com/reference/vector/vector/front/
    the front and back methods of std::vector objects are undefined, if the
    vector is empty. Therefore also the methods front(), back(), is_closed()
    have undefined behaviour on empty WayNodeListst.
    I hope this doesn't cause problems.

osmium/osm/bounds.hpp(60):
    Is there a reason, that no non-const getter for bottom_left and
top_right is implemented?
    If you want to change the bounds you can only use the extend() method.

Best regards,

Johannes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/dev/attachments/20130816/a14d4501/attachment.html>