6 ms·

I'm only a bit in but I've already found myself disagreeing with some of these for example drawline(int,int,int,int); // obscure drawline(Point,Point

by okasaki 11y ago

I'm only a bit in but I've already found myself disagreeing with some of these for example drawline(int,int,int,int); // obscure drawline(Point,Point); // clearer this is only "clearer" until you find yourself at the very common case where you use several libraries where each defines its own "Point". Now you're writing something like drawline(Point(vec.x1, vec.y1), Point(vec.x2, vec.y2)) Pretty ugly! And of course drawline(Point, Point) isn't clearer at all imo. What's the coordinate system? What are the units? Any performance notes on this function? Etc. I'll be checking the documentation the first time anyway. void do_something(vector<string>& v) { string val; cin>>val; // ... int index = 0; // bad for(int i=0; i<v.size(); ++i) if (v[i]==val) { index = i; break; } // ... } this is shown to be inferior to the find() version. Again, it's only inferior until the very common case where you're doing something else with the index, which is pretty common during debugging. Then you'll be rewriting the above "bad" version anyway. Similar problems other higher order functions like for_each().

TillE 11y ago
I don't enjoy converting stuff between different formats for different libraries either, but sticking associated bits of data together really does make sense. With a function that just takes four integers, you can't easily distinguish between two Points or a Rect, for example. Plus, a lot of problems are solved by namespacing and something like this: typedef std::tuple<int, int> Point;
Jyaif 11y ago
> What's the coordinate system? What are the units? Good question. Where are you going to look that up? In the documentation of drawline? Should drawrect, drawcircle, etc..., also include documentation about the coordinate system/units? Of course not, that information should be centralized. That's where Point come in. And that's just a detail, the main advantage of using a class is that is abstract details. In the Point class, you may want to switch from int to int64_t without having to revisit all the methods.
- ajross 11y ago
  That's the textbook explanation for this stuff, and matches what we see in the document. But I'm with the grandparent: this sort of pedantry fails badly in real-world code. What happens when you want to combine that rectangle implementation with someone else's geometry library which has its own notion of Point? This kind of nonsense is just a recipe for everyone spending lines of code and neural cycles on a bunch of junk that will just be unpacked back into the underlying representation anyway in order to be used in the real world. Don't do it. Simplicity trumps almost everything in real engineering. Abstract where abstractions add real value (which usually means "hide non-trivial amounts of code) and not where they do nothing but "document" stuff.
  - ThatGeoGuy 11y ago
    Isn't this the exact argument for why C++ has namespaces? I would assume that you could easily just do the following: void drawLine(lib1::Point, lib1::Point); void drawRectangle(lib2::Point, lib2::Point); In this case the usage is clear (and IIRC I think most IDE's will avoid omitting lib1:: and lib2:: in compiled code even if you do). The whole point of namespacing is to avoid the name collision issues that are (were?) inherent to C.
    - conistonwater 11y ago
      I think the point may have been to call a function in one library with the arguments of types from the other library, so that you need to write less glueing code. But in that case, you don't actually know that the point types are compatible (they might be), especially since their internal details can change. I don't think this is about name collision.
      - ajross 11y ago
        Exactly. The point is that the abstracted Point hides implementation details like the underlying representation, which is good as far as it goes because it prevents goofs like overflow and trucation due to incorrect type conversions, etc... But the second you need to match that abstraction to the code from some other developer which doesn't use it, you need that underlying representation back again. And this kind of mismatch isn't an uncommon edge case, it's the pervasive truth of modern development. Basically: C++ can't win here, and it would be better not to try IMHO.
gavazzy 11y ago
You can use an initializer list ctor for point: drawline({x0,x1}, {x2,x3})
bla2 11y ago
I agree, these are too opinionated to be unconditionally useful.
nhaehnle 11y ago
> drawline(Point(vec.x1, vec.y1), Point(vec.x2, vec.y2)) True, though you can usually reduce the pain by implementing appropriate conversion operators and/or implicit constructors. > Again, it's only inferior until the very common case where you're doing something else with the index, which is pretty common during debugging. You can use subtraction to obtain the index from the iterators in the better version. In this case, find really is superior.
RotsiserMho 11y ago
Someone mentioned using an initializer list, which is a pretty good option if you don't want to be explicit. However, writing a small helper function or class to automatically (and using constexpr, at compile time) convert between different kinds of points can achieve the same thing without losing the contextual benefits.
a_t48 11y ago
At my job, we have namespace::MathConvert() which takes instances of one type, and returns const refs to other types (assuming they are binary compatible), for the case of one Point type to another.
devbug 11y ago
Regarding drawline You can always define an implicit constructor: Point::Point(const Library::Point &p) Of course, you're now hiding a ticking time-bomb from a the next maintainer of that code. If you absolutely want the latter case, overloading is your friend: drawline(int x1, int y1, int x2, int y2); drawline(Point p1, Point p2) { drawline(p1.x, p1.y, p2.x, p2.y); } This, in my opinion, is the most conducive to refactoring (and modularizing code). As an aside, you really should be doing: drawline => drawlines "Where's the one, there's many."
Alphasite_ 11y ago
Wouldn't this be an argument of rising auto type conversions?
tdicola 11y ago
If you're working with a lot of different point types then it probably makes sense to define your own point type that provides type conversion to all the other point types used in your codebase. That way you're not coupled to one library's point type in your own code and can add extra functions, etc. to the point class that you might need. You can also explicitly control stuff like what frame of reference your coordinate system is in and your type conversion functions handle converting to different frames of reference for other point types.
randomprog 11y ago
> What's the coordinate system? What are the units? That information can be encoded into the class type, in which case you get compile time checking. I wrote a blog post about this some time ago: http://www.randomprogramming.com/2014/06/quote-of-the-week-type-systems-and-unit-tests/ http://www.randomprogramming.com/2014/06/quote-of-the-week-t... The summary is that we had two different coordinate systems in play and had many bugs caused by passing values in the wrong coordinate system around. By making the different coordinate systems different classes everything became explicit and checked by the compiler. We completely eliminated a whole class of bugs from our program. The great thing about using an int is that it can be anything (integer). It can be an x coordinate, or the number of eggs or a selection between multiple options. The bad thing about using an int is that it can be anything, and sooner or later you will pass the number of eggs into a function that really wants an x coordinate.
- fho 11y ago
  > That information can be encoded into the class type This. I did the very same thing when dealing with some code that was frequently converting between three coordinate systems. Using Haskell code duplication was close to non existent and still type-safe. Not wanting to sound too fan-boyish ... but that was one of the first times that I realized that a good type-system can help you prevent whole classes of bugs.
- RogerL 11y ago
  I prefer having member functions that convert to a different value, and friend functions that create instances from another type: Distance d = feet(7); d += meters(3); std::cout << d.yards(); Distance d2 = 3; // I won't compile This assumes that you want to mix units, not defeat it. It all ends up being very efficient; use MKS for the underlying representation. You can multiply, add, and so on, quite efficiently; the only conversions happen on input and output.
pjtr 11y ago
> several libraries where each defines its own "Point". Same problem where each library defines its own "String". Would be nice if the standard libraries would define such basic types and everyone would just use the same ones. This might work better without OO kitchen sink approach to types. Just standardize a simple data layout or interface and leave the possibility for libraries to define their own functionality in extension methods.
- maxlybbert 11y ago
  You do realize that std::string is 20+ years old now, right?
  - deleted 11y ago
    [deleted]
  - the_why_of_y 11y ago
    The problem with std::string is, it was useless in the real world because its interface is basically unsuitable for handling Unicode. That's why every project that cares about i18n had to define its own string class to get anything done. Only with C++11 did they add std::u16string, which is at least suitable for the (horrible) UTF16 encoding. Maybe we'll get an UTF8 capable std::string class in another 20 years.
    - maxlybbert 11y ago
      You've always had the ability to put utf-8 data in std::strings, but you have to keep track of which strings are encoded. C++11 adds functions to convert to utf-8 (and utf-16 and utf-32), http://en.cppreference.com/w/cpp/locale/codecvt http://en.cppreference.com/w/cpp/locale/codecvt .
  - pjtr 11y ago
    Sure. That was kind of my point. Still, as Herb Sutter asked at CppCon 2015: "How many of you have your own homegrown string types at your companies? ... Don't lie. ... Right. And those of you who didn't put up your hand and are just bashful, ... we know we have them." [1] [1] https://youtu.be/hEx5DNLWGgA?t=1373 https://youtu.be/hEx5DNLWGgA?t=1373
rleigh 11y ago
Note that with a long list of int values, it's also fairly easy to get the order wrong. Having a proper point type makes this less likely, as well as making the intent explicit.
nn3 11y ago
And when something goes wrong with the template version you often get an incomprehensible error message. Not the case with the for loop.
unfamiliar 11y ago
For me, I don't care that drawline(Point(vec.x1, vec.y1), Point(vec.x2, vec.y2)) is more or less clear. I mainly care about the fact that you can't easily tell which of these is correct: drawline(Point(vec.x1, vec.y1), Point(vec.x2, vec.y2)) drawline(Point(vec.x1, vec.x2), Point(vec.y2, vec.y2))

by okasaki 11y ago

TillE 11y ago

I don't enjoy converting stuff between different formats for different libraries either, but sticking associated bits of data together really does make sense. With a function that just takes four integers, you can't easily distinguish between two Points or a Rect, for example. Plus, a lot of problems are solved by namespacing and something like this: typedef std::tuple<int, int> Point;

Jyaif 11y ago

> What's the coordinate system? What are the units? Good question. Where are you going to look that up? In the documentation of drawline? Should drawrect, drawcircle, etc..., also include documentation about the coordinate system/units? Of course not, that information should be centralized. That's where Point come in. And that's just a detail, the main advantage of using a class is that is abstract details. In the Point class, you may want to switch from int to int64_t without having to revisit all the methods.

ajross 11y ago
That's the textbook explanation for this stuff, and matches what we see in the document. But I'm with the grandparent: this sort of pedantry fails badly in real-world code. What happens when you want to combine that rectangle implementation with someone else's geometry library which has its own notion of Point? This kind of nonsense is just a recipe for everyone spending lines of code and neural cycles on a bunch of junk that will just be unpacked back into the underlying representation anyway in order to be used in the real world. Don't do it. Simplicity trumps almost everything in real engineering. Abstract where abstractions add real value (which usually means "hide non-trivial amounts of code) and not where they do nothing but "document" stuff.
- ThatGeoGuy 11y ago
  Isn't this the exact argument for why C++ has namespaces? I would assume that you could easily just do the following: void drawLine(lib1::Point, lib1::Point); void drawRectangle(lib2::Point, lib2::Point); In this case the usage is clear (and IIRC I think most IDE's will avoid omitting lib1:: and lib2:: in compiled code even if you do). The whole point of namespacing is to avoid the name collision issues that are (were?) inherent to C.
  - conistonwater 11y ago
    I think the point may have been to call a function in one library with the arguments of types from the other library, so that you need to write less glueing code. But in that case, you don't actually know that the point types are compatible (they might be), especially since their internal details can change. I don't think this is about name collision.
    - ajross 11y ago
      Exactly. The point is that the abstracted Point hides implementation details like the underlying representation, which is good as far as it goes because it prevents goofs like overflow and trucation due to incorrect type conversions, etc... But the second you need to match that abstraction to the code from some other developer which doesn't use it, you need that underlying representation back again. And this kind of mismatch isn't an uncommon edge case, it's the pervasive truth of modern development. Basically: C++ can't win here, and it would be better not to try IMHO.

gavazzy 11y ago

You can use an initializer list ctor for point: drawline({x0,x1}, {x2,x3})

bla2 11y ago

I agree, these are too opinionated to be unconditionally useful.

nhaehnle 11y ago

> drawline(Point(vec.x1, vec.y1), Point(vec.x2, vec.y2)) True, though you can usually reduce the pain by implementing appropriate conversion operators and/or implicit constructors. > Again, it's only inferior until the very common case where you're doing something else with the index, which is pretty common during debugging. You can use subtraction to obtain the index from the iterators in the better version. In this case, find really is superior.

RotsiserMho 11y ago

Someone mentioned using an initializer list, which is a pretty good option if you don't want to be explicit. However, writing a small helper function or class to automatically (and using constexpr, at compile time) convert between different kinds of points can achieve the same thing without losing the contextual benefits.

a_t48 11y ago

At my job, we have namespace::MathConvert() which takes instances of one type, and returns const refs to other types (assuming they are binary compatible), for the case of one Point type to another.

devbug 11y ago

Regarding drawline You can always define an implicit constructor: Point::Point(const Library::Point &p) Of course, you're now hiding a ticking time-bomb from a the next maintainer of that code. If you absolutely want the latter case, overloading is your friend: drawline(int x1, int y1, int x2, int y2); drawline(Point p1, Point p2) { drawline(p1.x, p1.y, p2.x, p2.y); } This, in my opinion, is the most conducive to refactoring (and modularizing code). As an aside, you really should be doing: drawline => drawlines "Where's the one, there's many."

Alphasite_ 11y ago

Wouldn't this be an argument of rising auto type conversions?

tdicola 11y ago

If you're working with a lot of different point types then it probably makes sense to define your own point type that provides type conversion to all the other point types used in your codebase. That way you're not coupled to one library's point type in your own code and can add extra functions, etc. to the point class that you might need. You can also explicitly control stuff like what frame of reference your coordinate system is in and your type conversion functions handle converting to different frames of reference for other point types.

randomprog 11y ago

> What's the coordinate system? What are the units? That information can be encoded into the class type, in which case you get compile time checking. I wrote a blog post about this some time ago: http://www.randomprogramming.com/2014/06/quote-of-the-week-type-systems-and-unit-tests/ http://www.randomprogramming.com/2014/06/quote-of-the-week-t... The summary is that we had two different coordinate systems in play and had many bugs caused by passing values in the wrong coordinate system around. By making the different coordinate systems different classes everything became explicit and checked by the compiler. We completely eliminated a whole class of bugs from our program. The great thing about using an int is that it can be anything (integer). It can be an x coordinate, or the number of eggs or a selection between multiple options. The bad thing about using an int is that it can be anything, and sooner or later you will pass the number of eggs into a function that really wants an x coordinate.

fho 11y ago
> That information can be encoded into the class type This. I did the very same thing when dealing with some code that was frequently converting between three coordinate systems. Using Haskell code duplication was close to non existent and still type-safe. Not wanting to sound too fan-boyish ... but that was one of the first times that I realized that a good type-system can help you prevent whole classes of bugs.
RogerL 11y ago
I prefer having member functions that convert to a different value, and friend functions that create instances from another type: Distance d = feet(7); d += meters(3); std::cout << d.yards(); Distance d2 = 3; // I won't compile This assumes that you want to mix units, not defeat it. It all ends up being very efficient; use MKS for the underlying representation. You can multiply, add, and so on, quite efficiently; the only conversions happen on input and output.

pjtr 11y ago

> several libraries where each defines its own "Point". Same problem where each library defines its own "String". Would be nice if the standard libraries would define such basic types and everyone would just use the same ones. This might work better without OO kitchen sink approach to types. Just standardize a simple data layout or interface and leave the possibility for libraries to define their own functionality in extension methods.

maxlybbert 11y ago
You do realize that std::string is 20+ years old now, right?
- deleted 11y ago
  [deleted]
- the_why_of_y 11y ago
  The problem with std::string is, it was useless in the real world because its interface is basically unsuitable for handling Unicode. That's why every project that cares about i18n had to define its own string class to get anything done. Only with C++11 did they add std::u16string, which is at least suitable for the (horrible) UTF16 encoding. Maybe we'll get an UTF8 capable std::string class in another 20 years.
  - maxlybbert 11y ago
    You've always had the ability to put utf-8 data in std::strings, but you have to keep track of which strings are encoded. C++11 adds functions to convert to utf-8 (and utf-16 and utf-32), http://en.cppreference.com/w/cpp/locale/codecvt http://en.cppreference.com/w/cpp/locale/codecvt .
- pjtr 11y ago
  Sure. That was kind of my point. Still, as Herb Sutter asked at CppCon 2015: "How many of you have your own homegrown string types at your companies? ... Don't lie. ... Right. And those of you who didn't put up your hand and are just bashful, ... we know we have them." [1] [1] https://youtu.be/hEx5DNLWGgA?t=1373 https://youtu.be/hEx5DNLWGgA?t=1373

rleigh 11y ago

Note that with a long list of int values, it's also fairly easy to get the order wrong. Having a proper point type makes this less likely, as well as making the intent explicit.

nn3 11y ago

And when something goes wrong with the template version you often get an incomprehensible error message. Not the case with the for loop.

unfamiliar 11y ago

For me, I don't care that drawline(Point(vec.x1, vec.y1), Point(vec.x2, vec.y2)) is more or less clear. I mainly care about the fact that you can't easily tell which of these is correct: drawline(Point(vec.x1, vec.y1), Point(vec.x2, vec.y2)) drawline(Point(vec.x1, vec.x2), Point(vec.y2, vec.y2))