Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

During a recent project I've been working on, I've had to use a lot of functions that kind of look like this:

static bool getGPS(double plane_latitude, double plane_longitude, double plane_altitude,
                       double plane_roll, double plane_pitch, double plane_heading,
                       double gimbal_roll, double gimbal_pitch, double gimbal_yaw, 
                       int target_x, int target_y, double zoom,
                       int image_width_pixels, int image_height_pixels,
                       double & Target_Latitude, double & Target_Longitude, double & Target_Height);

So I want to refactor it to look something like this:

static GPSCoordinate getGPS(GPSCoordinate plane, Angle3D planeAngle, Angle3D gimbalAngle,
                            PixelCoordinate target, ZoomLevel zoom, PixelSize imageSize)

This appears to me to be significantly more readable and safe than the first method. But does it make sense to create PixelCoordinate and PixelSize classes? Or would I be better off just using std::pair<int,int> for each. And does it make sense to have a ZoomLevel class, or should I just use a double?

My intuition behind using classes for everything is based on these assumptions:

  1. If there are classes for everything, it would be impossible to pass a ZoomLevel in where a Weight object was expected, so it would be more difficult to provide the wrong arguments to a function
  2. Likewise, some illegal operations would cause compile errors, such as adding a GPSCoordinate to a ZoomLevel or another GPSCoordinate
  3. Legal operations will be easy to represent and typesafe. i.e subtracting two GPSCoordinates would yield a GPSDisplacement

However, most C++ code I've seen uses a lot of primitive types, and I imagine there must be a good reason for that. Is it a good idea to use objects for anything, or does it have downsides that I am not aware of?

share|improve this question
8  
"most C++ code I've seen uses a lot of primitive types" - two thoughts come to mind: a lot of C++ code may have been written by programmers familiar with C which has weaker abstraction; also Sturgeon's Law –  congusbongus Mar 28 '13 at 8:19
3  
And I think it's because primitive types are simple, straightforward, fast, sleek and efficient. Now in the OP's example, it's definitely a good idea to introduce some new classes. But the answer to the title question (where it says "everything") is a resounding no! –  Mr Lister Mar 28 '13 at 10:24
    
What does the c++istas say about using typedef for those single doubles? –  Max Mar 28 '13 at 10:32
1  
@CongXu: I had almost the same thought, but more in the sense "a lot of code in any language may have been written by programmers having problems with building abstractions at all". –  Doc Brown Mar 28 '13 at 12:22
1  
As the saying goes "if you have a function with 17 parameters, you probably missed a few". –  Joris Timmermans Mar 28 '13 at 15:19

5 Answers 5

up vote 24 down vote accepted

Yes, definitely. Functions/methods that take too many arguments is a code smell, and indicates at least one of the following:

  1. The function/method is doing too many things at once
  2. The function/method requires access to that many things because it's asking, not telling or violating some OO design law
  3. The arguments are actually closely related

If the last one is the case (and your example certainly suggests so), it's high time to do some of that fancy-pants "abstraction" that the cool kids were talking about oh, just some decades ago. In fact, I'd go further than your example and do something like:

static GPSCoordinate getGPS(Plane plane, Angle3D gimbalAngle, GPSView view)

(I am not familiar with GPS so this is probably not a correct abstraction; for instance, I don't understand what zoom has to do with a function called getGPS.)

Please prefer classes like PixelCoordinate over std::pair<T, T>, even if the two have the same data. It adds semantic value, plus a bit of extra type safety (compiler will stop you from passing a ScreenCoordinate to a PixelCoordinate even though both are pairs.

share|improve this answer
1  
not to mention you can pass the larger structs by reference for that little bit of micro-optimization all the cool kids are trying to do but really shouldn't –  ratchet freak Mar 28 '13 at 9:05

Disclaimer: I prefer C to C++

Instead of classes, I would use structs. This point is pedantic at best, since structs and classes are nearly identical (see here for a simple chart, or this SO question), but I think there is merit in the differentiation.

C programmers are familiar with structs as blocks of data that have some greater meaning when grouped, whereas when I see a clasas, I assume there's some internal state and methods to manipulate that state. A GPS coordinate doesn't have state, just data.

From your question, it seems this is exactly what you're looking for, a general container, not a stateful structure. As the others have mentioned, I wouldn't bother putting Zoom into a class of its own; I personally hate classes built to hold a data type (my professors love creating Height and Id classes).

If there are classes for everything, it would be impossible to pass a ZoomLevel in where a Weight object was expected, so it would be more difficult to provide the wrong arguments to a function

I don't think this is a problem. If you reduce the number of parameters by grouping the obvious things like you've done, the headers should be enough to prevent those problems. If you're really worried, separate the primatives with a struct, which should give you compile errors for common mistakes. It's easy to swap two parameters next to each other, but harder if they're farther apart.

Likewise, some illegal operations would cause compile errors, such as adding a GPSCoordinate to a ZoomLevel or another GPSCoordinate

Good use of operator overloads.

Legal operations will be easy to represent and typesafe. i.e subtracting two GPSCoordinates would yield a GPSDisplacement

Also good use of operator overloads.

I think you're on the right track. Another thing to consider is how this fits into the rest of the program.

share|improve this answer

Using dedicated types has the advantage that it's much harder to screw up the order of arguments. The first version of your example function, for example, begins with a chain of 9 doubles. When someone uses this function, it is very easy to screw up the order and pass the gimbal angles in place of the GPS coordinates and vice versa. This can lead to very strange and hard to trace bugs. When you pass only three arguments with different types instead, this error can be catched by the compiler.

I would in fact consider to go a step further and introduce types which are technically identical but have a different semantic meaning. For example by having a class PlaneAngle3d and a separate class GimbalAngle3d, even though both are a collection of three doubles. This would make it impossible to use one as the other, unless it's intentional.

I would recommend using typedefs which are just aliases for primitive types (typedef double ZoomLevel) not just for that reason, but also for another: It easily allows you to change the type later. When you get the idea one day that using float could be just as good as double but could be more memory- and CPU-efficient, you just have to change this in one place, and not in dozends.

share|improve this answer
    
+1 for the typedef suggestion. Gets you the best of both worlds: primitive types and objects. –  Karl Bielefeldt Mar 28 '13 at 13:03
    
It's no harder to change the type of a class member than a typedef; or did you mean compared to primitives? –  MaHuJa Oct 28 '13 at 14:09

Fine answers here already, but there is one thing I would like to add:

Using PixelCoordinate and PixelSize class makes things very specific. A general Coordinate or Point2D class may probably better suit your needs. A Point2D should be a class implementing the most common point/vector operations. You could implement such a class even generically (Point2D<T> where T may be int or double or something else).

2D point/vector operations are so very common for lots of 2D geometry programming tasks that to my experience such a decision will increase highly the chance of reusing existing 2D operations. You will avoid the need for re-implementing them for each PixelCoordinate, GPSCoordinate, "Whatsover"-Coordinate class again and again.

share|improve this answer
1  
you can also do struct PixelCoordinate:Point2D<int> if you want proper type safety –  ratchet freak Mar 28 '13 at 13:32

So I want to refactor it to look something like this:

static GPSCoordinate getGPS(GPSCoordinate plane, Angle3D planeAngle, Angle3D gimbalAngle,
                        PixelCoordinate target, ZoomLevel zoom, PixelSize imageSize)

This appears to me to be significantly more readable and safe than the first method.

It is [more readable]. It is also a good idea.

But does it make sense to create PixelCoordinate and PixelSize classes?

If they represent different concepts, it makes sense (that is, "yes.").

Or would I be better off just using std::pair for each.

You could start by doing typedef std::pair<int,int> PixelCoordinate, PixelSize;. This way, you cover the semantics (you're working with pixel coordinates and pixel sizes, not with std::pairs in your client code) and if you need to extend, you simply replace the typedef with a class and your client code should require minimal changes.

And does it make sense to have a ZoomLevel class, or should I just use a double?

You could typedef it as well, but I would use a double until I'd need functionality associated with it that doesn't exist for a double (for example, having a ZoomLevel::default value would require that you define a class, but until you have anything like that, keep it a double).

My intuition behind using classes for everything is based on these assumptions [omitted for brevity]

They are strong arguments (you should always strive to make your interfaces easy to use correctly and difficult to use incorrectly).

However, most C++ code I've seen uses a lot of primitive types, and I imagine there must be a good reason for that.

There are reasons; in lots of cases though, those reasons are not good. One valid reason to do this may be performance, but that means that you should see this kind of code as an exception, not as a rule.

Is it a good idea to use objects for anything, or does it have downsides that I am not aware of?

It is generally a good idea to make sure that if your values always appear together (and make no sense apart) you should group them together (for the same reasons you mentioned in your post).

That is, if you with coordinates that always have x, y, and z, then it's much better to have a coordinate class than transmit three parameters everywhere.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.