Question Class Constructors, Setters, and Getters.

Pages: 12
closed account (Ezyq4iN6)
I had a question about accepted practices for class structure. As an example I will create a vehicle class with a length and width that don't change, and x and y coordinates, velocity, and angle that do change, but also need to be defined in order to use the object.

From what I have been told, I should put everything that I need for the object to work in the constructor. However, I can see this getting really long if I had more variables. Also, I was told that readability is important, so I am tempted to set every value with setters so that anyone reading my code knows exactly what it does.

I have also considered a hybrid, where I set only the length and width in the constructor (because they don't change), and then set the others with an initializer function, but then again that just feels like a second constructor.

What are the accepted ways to create classes, in specific like the one I have described above and will put the code for below, so that I can start off with good practices? Please note that ALL options will have setters for changing values, but I only included them in the example where they are used to initialize the values.


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
  // All in constructor option.
  class Vehicle
  {
    public:
      Vehicle(int length, int width, int x, int y, int velocity, int angle);
    private:
      const int _length;
      const int _width;
      int _x;
      int _y;
      int _velocity;
      int _angle;
   }

   // Setter option.
  class Vehicle
  {
    public:
      Vehicle();
      void setLength(length);
      void setLength(width);
      void setLength(x);
      void setLength(y);
      void setLength(velocity);
      void setLength(angle);
    private:
      const int _length;
      const int _width;
      int _x;
      int _y;
      int _velocity;
      int _angle;
   }

   // Hybrid option.
  class Vehicle
  {
    public:
      Vehicle(length, width);
      init(x, y, velocity, angle);
    private:
      const int _length;
      const int _width;
      int _x;
      int _y;
      int _velocity;
      int _angle;
   }


Thank you for helping this new coder.
Last edited on
A well-designed object should be easy to use correctly, and difficult to use incorrectly.

If you force the user to have to call a half-dozen functions to create an object, that's a half-dozen opportunities to get it wrong.

A well-designed object should be valid for its entire lifetime. It should ideally be impossible, or at least very hard, for the user to have one that's in a bad state. Valid at construction, valid until destruction. I want to be able to trust my objects to be in a good state and to look after themselves.

Please note that ALL options will have setters for changing values, but I only included them in the example where they are used to initialize the values.

Why? Why bother having private variables if you're just going to let the user touch them directly anyway? A well-designed object should provide only the functionality the user needs to use it directly. If you have a class that turns out to be no more than a collection of variables, redesign or at least just make it a plain struct with no functions so the user can see that it's no more than a collection of variables.


I see that some of your class variables are const, so you can't have set functions for them anyway.


Having a two-stage constructor and then init, purely because it looks more pretty than a single constructor, will get you beaten up in the car park at work.
Last edited on
Vehicle(int length, int width, int x, int y, int velocity, int angle);

I think this looks fine. If you need to change it later, you can.

You might want to consider wrapping parameters that belong together and get changed together into their own class.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
// Example program
#include <iostream>
#include <string>

struct Point {
    Point()
    : x(0), y(0) { }
    Point(int x, int y)
    : x(x), y(y) { }
    int x;
    int y;
};

struct BoundingBox {
    BoundingBox(const Point& point, int width, int length)
    : position(point), width(width), length(length) { }
    
    Point position;
    int width;
    int length;
};

class Car {
  public:
    Car(const BoundingBox& bb, int velocity)
    : frame(bb), velocity(velocity) { }
    
    BoundingBox frame;
    
    int getCurrentSpeed()
    {
        return velocity;   
    }
    
    void accelerate()
    {
        velocity += 10;
        if (velocity > 120)
            velocity = 120;
    }
    
    void brake()
    {
        velocity -= 20;
        if (velocity < 0)
            velocity = 0;
    }
    
    
  private:
    int velocity;
};

int main()
{
    Car car(
        BoundingBox(Point(30, 20), 50, 200),
        60
    );
    
    for (int i = 0; i < 10; i++)
    {
        car.accelerate(); 
        std::cout << car.getCurrentSpeed() << std::endl;
    }
    for (int i = 0; i < 10; i++)
    {
        car.brake();
        std::cout << car.getCurrentSpeed() << std::endl;
    }
}

70
80
90
100
110
120
120
120
120
120
100
80
60
40
20
0
0
0
0
0

Just an idea.

I'm not saying you are, but don't freeze for too long due to overthinking design. Especially as a beginner (I assume). Don't get me wrong -- design is very important in the long run. But as a beginner, just focus on writing code. As you write more code and have to keep track of a larger project with multiple files and interactions between different components, you'll start to get a better understanding of what works and what doesn't work.

Edit: Updated code to allow car to accelerate and brake.

PS: While the intention is still perfectly understood, in English I would consider it wrong to have a variable called "velocity" when it's a scalar (speed).
Personally, I would change it to be a vector (not std::vector, the math vector) to allow your car to go in 2D or 3D directions.

PPS: As a guide, one thing you can do is look at other people's code for actual large projects or libraries. Look on sites that host source code, like github or somewhere.
Last edited on
Edit: I didn't see the other replies while writing mine.

Hi,

The purpose of a constructor is to initialize all member variables, this is best done with a member initialization list:

https://en.cppreference.com/w/cpp/language/initializer_list

This is efficient because it initializes everything once, rather than the members being given default values implicitly by the compiler, then being set again when you call your setter functions, or otherwise explicitly set them.

I prefer to put Arg on the end of the parameter names, instead of marking the member variables in some way - like the underscore you are using.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
public:
     Vehicle::Vehicle(const int lengthArg, // put comments here if necessary
                               const int widthArg,   // const here , but not in function declaration
                               const int xArg,         // this function is not changing the argumets
                               const int yArg, 
                               const int velocityArg, 
                               const int angleArg)
  :  // colon introduces member initializer list
  length{lengthArg},  // brace initialization, compiler will complain about any narrowing, eg double to int
  width(widthArg),  // change the class declaration: remove underscores so this works
  x{xArg},
  y{yArg},
  velocity{velocityArg},
  angle{angleArg}
{
  // function body, do error checking / validation here or call another function that does that
} 


In C++ the object is not deemed to have been created until the closing brace of a constructor is reached, so if there is a problem with the validity of the data, then one would throw an exception. That may be a bit advanced right now, but at least you are aware of it :+)

With your types: int is not going to work well when there are angles and coordinates, perhaps double would be better? Even if the coords have to int, say for a graphics library function, it is better to calculate them with double, then convert to int at the end.

With getters and setters: if your class has them both for each member variable, then it may as well have everything public - which is bad. Instead consider what your interface (the public functions) is going to do: there are going to be changes to velocity and angle, so write a function that takes these values as arguments, and sets the x and y values within that function. Remember that member functions have direct access to member variables.

When you come to output some info, you can have a function to do that too, rather than calling a bunch of get functions to do the same thing.

Ideally the Vehicle shouldn't need to know anything about position or velocity details. Think of it like the the Vehicle class storing only info about the physical attributes. One could have another class which stores vehicle position and velocity. The x and y could belong to a Point class, and in physics velocity is speed and direction.

Here is some more reading:

https://en.wikipedia.org/wiki/SOLID

Good Luck !!
Last edited on
closed account (Ezyq4iN6)
Sorry for the late response, but my previous attempts to respond resulted in logging me out.

@Repeater
Thanks for the good set of rules for making a object/class. As to you asking "why bother having private variables", that is a question I often asked myself, as several example classes I looked at had a setter and getter for almost or all private variables. I simply figured they did this since I read the accepted status quo on classes was to not let people directly access properties.

The struct idea is good, and I guess I could use it instead, since I read you can have functions in structs too. Is a struct essentially a class, only things are innately public?

You're right about the set functions and the const values. I forgot to leave out the set functions for them.

@Ganado
I never though about sud-dividing my car into different structs and classes, but that may make it easier to read, or at least have one part not mess up another.

Your post was the first time I have seen the ':' symbol used like that. Based on TheIdeasMan's post, I assume that is an initializer list? Code-wise, what are the pluses and minuses to that format vs. the following format:

1
2
3
4
5
6
7
8
9
10
// What you had.
Point(int x, int y)
    : x(x), y(y) { }

// What I normally do.
Point(int x, int y)
{
  x = x;
  y = y;
}


Also, will using the same variable names as arguments and in the class cause a problem, or does the compiler sort that out?

That's a good idea about changing velocity to speed. It makes more sense. I'll take a look at github as well. I have started an account there so I can store my code, but still don't quite understand how it works. Lots of nice examples though.

@TheIdeasMan
The initializer list is something I have never seen before. As I mentioned in my reply to Ganado, I have always done/seen it done like my example I included. What makes the initializer list the preferred option?

I agree on the underscore. I only use it because I was told to. The Arg works too, and is easier to see. Also, will switch my ints to doubles because I will definitely need non-integers. I'm using SDL2 for graphics right now, so I'll have to remember to convert them back to ints like you said.

Since velocity and angle are going to be changed, is it fine to make them public and simply change the properties directly instead of using a set function? I figure that could save a few lines. I guess my whole thing with set/get functions was that when you build a class you need to make it completely accessible for anyone else using it, because they may want to use it a different way, etc., hence my confusion about standards and good practices that lead me post the original question.

Thanks for the links too.
Is a struct essentially a class, only things are innately public?

Yes. The inheritance is also public by default.

When you get past toy examples, you'll design classes that contain internal variables that the user should not and will not have access to. Then it will become clearer.

Also, will using the same variable names as arguments and in the class cause a problem, or does the compiler sort that out?

The compiler will let you "shadow" variables (which is the effect of hiding a variable that came from from an outer scope, by creating a new variable with the same name inside the scope). Anyone who ever has to read your code will be much less forgiving, including yourself in six months' time when you're trying to decipher your own cryptic code. The compiler doesn't care how your code is formatted, or what you name your variables; you're formatting your code and naming your variables to help humans understand it. Make it easy for them.
Last edited on
struct came in WAY before classes (its from C) and was originally only able to hold data (no methods). Its grown to catch up to classes, making the differences between the two tools very small. They were not always redundant, and too much code uses struct to make it obsolete.
I assume that is an initializer list?

Initializer lists can be very useful, though if we're just talking about initializing types of ints and other primitive types, the benefits are only minor.

First, yes, using the initializer list prevents you from having to do "this->" syntax when the parameter name is the same as the class member variable name.

When the body of the constructor is reached, all variable initialization will have already happened. In the body of the constructor, class variables cannot be initialized, they can only be assigned. This includes any base class initialization: If you don't specify the constructor of the base-class, it will be default initialized.

Here's an example of program that won't compile unless you use the initialization list.
The reason here is that the base-class constructor only has 1 (non-default) constructor available, but without an initializer list, only a non-existent default constructor could be called.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
class BaseClass {
  public:
    // only one, 1-arg constructor
    BaseClass(int x) { } 
};

class Tumultuous : public BaseClass {
  public:
    Tumultuous()
    //: BaseClass(42)
    {
       
    }
};

int main()
{
    Tumultuous t;
}
Uncomment the ": BaseClass(42)" line to get it to compile.

Here's another example of something that won't compile unless you use the initializer list.
The reason here is that const variables can only be initialized, and never assigned to.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
class Tumultuous {
  public:
    Tumultuous()
    //: banana(42)
    {
       
    }
    
    const int banana;
};

int main()
{
    Tumultuous t;
}
Uncomment ": banana(42)" to get it to compile.

Third, references must always be initialized using the initializer list, because it's illegal to have a reference that isn't referencing anything.

All the boring details and edge cases are found here: https://en.cppreference.com/w/cpp/language/initializer_list

1
2
3
4
5
6
// What I normally do.
Point(int x, int y)
{
  x = x;
  y = y;
}


Also, will using the same variable names as arguments and in the class cause a problem, or does the compiler sort that out?

Your example is slightly wrong, because you're just assigning the argument to itself, leaving your actual class variable unassigned. When two variables are ambiguous, C++ will go for the variable in the inner-most scope first.
In other words, if you want both variables to be called the same thing, you have to do this:
1
2
3
4
5
Point(int x, int y)
{
  this->x = x;
  this->y = y;
}

this->x refers to the class member
x referes to the argument.
Last edited on
Since velocity and angle are going to be changed, is it fine to make them public and simply change the properties directly instead of using a set function?


No, member variables should still be private. Use the interface (public functions) to manipulate them. An example of what I mean with a proper interface rather than get / set functions:

Say one has a CAD system, it can draw and manipulate various drawing entities like lines, circles, text etc. There are various operations like scale, rotate, move, stretch. These operations are the public interface. We don't have MoveX(), MoveY(), MoveZ() etc, just Move() which will alter the values of X, Y, Z .

So it is possible to:

Initialize with one of the constructors;
Manipulate with interface functions;
Output values with operator<< or your own function.

Note I don't have a problem with get functions, but if one doesn't need them don't write them.

Another thing: it's fine for something like Point to be a simple struct (public by default), because often a Point variable will be a member of a class with private access. If Point were a class with private X,Y,Z , in order to change Centre, I would find it a pain to have to call a member function of Point while already inside a member function of Circle say. If it's a plain struct, I could change it directly.

1
2
3
4
5
6
7
8
9
10
11
12
struct Point {
  double X;
  double Y;
};

class Circle {
  private:
  Point Centre;
  double  Radius;

  // rest of class omitted for brevity
}


With the the this-> , another reason I prefer to put Arg, so I don't have to have this-> everywhere.

What makes the initializer list the preferred option?


With member initialization lists, remember that initialization can be complex with more than 1 base class, classes themselves made of complicated objects ; and containers could have millions of items - any inefficiencies in initialization would be observable.

More reading, this one is GOLD :+)
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
https://isocpp.org/faq
Last edited on
closed account (Ezyq4iN6)
@Repeater
You make a good case for using different variable names. I can imagine readability would improve a lot by using different variable names, which I changed in my updated example further down.

@jonnin
Thanks for letting me know about struct. I was always curious why it was so similar to class, especially since it can use functions.

@Ganado
The initializer does seem really helpful, though I did rename my argument variables to help improve readability. I never knew about using, this->, but that is good to know. I also never considered the difference between initialization and assignment from a class standpoint. I guess that makes the initalizer more efficient.

Your example is with the base class really helped me understand more about how to work with inheritance. I was having a little trouble with it. I would never have thought to initialize the base class like that. I also didn't know that you needed to use it for consts, but I guess that makes sense being that they can't be modified later.

@TheIdeasMan
In my example below, is the struct designed the correct way? I mean, it seems private to the outside world, and just accessible to the class.

Also, I added _arg to the end of my arguments. Is that ok, or is camel case expected in C++? Sorry, I am trying to learn Python too, and I know they used underscores there. Also, I noticed in your initilization list you used {} instead of () like Ganado. What is the difference?

Also, it sounds like objects with multiple inheritance would need giant initialization lists. Is this the way it is done, or is there another way around it?

Thanks for the additional links. May take a bit to read and comprehend those.

Here is my updated code for my vehicle class/test based on everyone's great advice:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
#include <iostream>
using std::cout;
using std::endl;

class Vehicle
{
  struct Body
  {
    double width;
    double length;

    Body(double width_arg, double length_arg):
    width(width_arg), length(length_arg) {}
  };

  struct Location
  {
    double x;
    double y;

    Location(double x_arg, double y_arg):
    x(x_arg), y(y_arg) {}

  };

  const Body vehicle_body;
  static const double speed_change = 1.0;

  Location position;
  double speed;
  double angle;

  void report_speed()
  {
    if (speed == 0.0)
      cout << "The vehicle is stopped." << endl;
    else
      cout << "Vehicle's new speed: " << speed << " m/s" << endl;
  }


public:
  Vehicle(double length_arg, double width_arg, double x_arg, double y_arg,
          double speed_arg, double angle_arg):
          vehicle_body(length_arg, width_arg), position(x_arg, y_arg),
          speed(speed_arg), angle(angle_arg) {}
  void report()
  {
    cout << "Vehicle length: " << vehicle_body.length << " m" << endl;
    cout << "Vehicle width: " << vehicle_body.width << " m" << endl;
    cout << "Vehicle X Position: " << position.x << endl;
    cout << "Vehicle Y Position: " << position.y << endl;
    report_speed();
    cout << "Vehicle angle: " << angle << "\370" << endl;
    cout << "--------------------------" << endl;
  }
  void accelerate()
  {
    speed += speed_change;
    if (speed > 10.0)
      speed = 10.0;
    report_speed();
  }
  void brake()
  {
    speed -= speed_change;
    if (speed < 0.0)
      speed = 0.0;
    report_speed();
  }

};

int main()
{
  Vehicle car(4.9, 2.1, 100.0, 100.0, 0.0, 0.0);
  car.report();
  car.accelerate();
  car.accelerate();
  car.accelerate();
  car.brake();
  car.brake();
  car.brake();
  return 0;
}

Hi,

That looks OK - Good Work!!, but there are some things I would do differently:

If the Location struct was out side the class, it could be reused for other things.

It's a good idea to separate the class definition and the member function implementations, see this post:
http://www.cplusplus.com/forum/general/248394/#msg1094846

if (speed == 0.0)

That might not work for all doubles, they aren't represented exactly. What you need is a "Equal within a precision " function. If the absolute value of Value - expected value is less than precision, then it returns true In general stick to relational comparisons, not equality.

Is that ok, or is camel case expected in C++?


What you have done is fine, there are no expectations, people / organizations have personal preferences / coding standards and they all vary. The isocpp superfaq mentions things not to do. I like PascalCase, others like all lower with underscores - that's what the C++ STL does. I like PascalCase, so it doesn't look like STL.

Also, I noticed in your initilization list you used {} instead of () like Ganado. What is the difference?


Parentheses means direct initialization, braces are used for several different initializations. There are lots of ways to initialize:
There is an initialization section.
https://en.cppreference.com/w/cpp/language

That site might be hard to read & comprehend, because it is a technical reference based on the C++ standards. Stick with it though.

Look at the reference on this site too, the examples might be easier.

Also, it sounds like objects with multiple inheritance would need giant initialization lists. Is this the way it is done, or is there another way around it?


Note that multiple inheritance is different from several levels of direct inheritance. Multiple means inheritance from 2 or more bases classes at the same time. As in C inherits from A and B (multiple) versus C inherits from B which inherits from A.

Generally we try to avoid big inheritance trees, and be careful with multiple inheritance. There are other ways of doing things, like Design Patterns, but that is more advanced.

It's good you don't have using namespace std; but it also good to not have using std::cout; either, only because it's easy to have 20 of them at the start of the file (by using different containers and algorithms), that gets tiresome. Just put std:: before every std thing. That's what all the experienced coders do, you might as well start now :+)

Ideally, your code should be in it's own namespace/s. You might get brownie points for doing that :+)

With variable and function names, choosing meaningful names can lead to the code telling a story of what it does. This helps greatly for humans to understand it, especially if it's a maintenance coder (not you) years later.

With formatting:

If there are several arguments, I like to put them 1 per line, it's easier to read:

1
2
3
4
5
6
7
8
9
10
11
12
13
public:
  Vehicle(double length_arg,
               double width_arg, 
               double x_arg, 
               double y_arg,
               double speed_arg, 
               double angle_arg)
       :
          vehicle_body(length_arg, width_arg), 
          position(x_arg, y_arg),
          speed(speed_arg), 
          angle(angle_arg) 
{}


It looks nice if they are all lined up, tricky here because the font is not fixed width.

I do the same thing with std::cout when there are multiple things to print:

1
2
3
4
5
6
7
8
9
10
 void report()
  {
    std::cout << "Vehicle length: " << vehicle_body.length << " m\n"
                  << "Vehicle width: " << vehicle_body.width << " m\n" 
                  << "Vehicle X Position: " << position.x << "\n"
                  << "Vehicle Y Position: " << position.y << "\n"
    report_speed();
    std::cout << "Vehicle angle: " << angle << "\370\n" 
                  << "--------------------------" << "\n";
  }


Notice I didn't use std::endl (it's slow), just a \n instead.

Basically good use of vertical and horizontal white space and anything else to make the code pleasing to look at will be appreciated by all kinds of people reading the code, your boss for instance :+)

Have fun !!
Just put std:: before every std thing. That's what all the experienced coders do, you might as well start now :+)

Only because C++ is dumb and doesn't have proper "using"/"import" clauses like C#, due to its historic C-style #includes. And compiler errors relating to naming conflicts are notoriously hard to interpret. But yes, I do usually end up putting std:: before everything. I like how it separates the standard library from other user libraries. :) I would argue if only used on the implementation file scope, things like using std::cout; is fine... but it's not a big deal -- it's more important to have fun, as you said.

Edit:
Is that ok, or is camel case expected in C++?

Do what you like, the biggest thing here is to be consistent. Don't have one class member be snake_case, and then another right next to it be CamalCase.
Last edited on
Try to keep classes small and if they get too many variables, find a logical way to break it up and put it back together using has-a type design... for example a class that has 3 doubles for the x,y,z parts of something (this could just be a vector or typedef over a vector) -- then you would have 2 of those (one for xyz position, one for the HPR angles) and so on inside vehicle. wrap everything in 2s (2d) or 3s (3d) where it makes sense.

you don't have to do that, but I would much prefer to have a vector of positions than 3 distinct variables each with its own getter/setter.

there are hundreds of c++ coding conventions. There is a pretty standard one for printed books (it avoids excess whitespace which costs pages which costs money to print), another for opensource, another for using documentation tools (like javadocs), older ones like hungarian styles, and many more. Nearly every development shop has their own flavor. As an example, many say all uppercase constants, but our shop did not do that because so many constants in math/engineering are lower case (eg pi, e, ...) or mixed case. We prefer it to look like a math textbook.
Last edited on
Why are you storing the position using cartesian coordinates (x,y) but storing the velocity using polar coordinates (speed, angle)? I'd store them both as cartesian vectors. And when you think of it this way, it's natural to store the acceleration too:

1
2
3
4
5
6
7
8
9
10
struct Vector {
    int x, y;
    Vector(xArg=0, yArg=0) :
        x(xArg), y(yArg) {}
};
...
class Vehicle {
...
    Vector pos, vel, acc;  // position, velocity, acceleration
};


I'm in the minority here, but I think getters and setters are overrated. They certainly have their place in the world, but unless I know there's a good reason to have them, I prefer public data instead. It makes the code so much less verbose.
I'm in the minority here, but I think getters and setters are overrated. They certainly have their place in the world, but unless I know there's a good reason to have them, I prefer public data instead. It makes the code so much less verbose.

I am all for this as well. But he was asking about best practices, and unfortunately, the bloated G/S design has taken the mainstream. What I do is, if any variable needs a G/S, they all get one. If none do, they all don't get one. Needs means the setter for at least one variable does more than set just that value (could be validation, or a dependency update).
Last edited on
dhayden/jonnin, agreed, although personally I like to avoid the Vector(xArg=0, yArg=0) default value constructor because it allows for things like: Vector(3). (What makes X more special than Y?) I mean, if that makes sense to you, go for it, but for me, I like to avoid the asymmetry that causes.

As far as getters/setters, yeah I've seen some over-designed parts of codebases that use getters and setters and interfaces with "Class" and "ClassImpl" constructs, with layers upon layers of inheritance. bleh. Sometimes it helps, but other times I think it just increases the number of useless "nouns" you have to keep track of as you code. When you get into the guts of programming like that, it makes programming lose some of its magic. Just keep it simple, and use what you need (and if you start going into territory that looks sketchy, be sure to document WHY you're making such weird hierarchies).

C# Properties (and auto-properties) are nice in that you can make them look like variables to the end-user, but they can still have an implementation to update internal details -- so in C# there's almost no reason to have a public variable that isn't a property, in my experience.
Last edited on
closed account (Ezyq4iN6)
@TheIdeasMan

Based on your advice, here is what I changed (code at the bottom):
I moved Location and Body structs out of my class. I created a header file to separate class definitions and member function implementation. I put the speed range from -0.05 to 0.05 instead of == 0.0. A added std:: to all of the couts, which I put together like your example, and removed the endls. I added my class to a namespace. I put arguments and initializer arguments on separate lines.

I may have missed something. Also, thanks for the new links.

@jonnin
Thanks. I took a look at has-a on wikipedia, and while I'm not sure I fully understand, I get the point about breaking things up into more parts to make them easier to work with. Can you write me or point me to a short code example about the xyz position and hpr angle example you mentioned? I'm having a little trouble visualizing what you mean.

@dhayden
I can see what you mean by having everything in the same coordinate system, but because of they way I normally calculate things, I'm not sure I understand how to modify my calculations to fit the example you gave. Here is a code snippet of how I would normally calculate with position/velocity/acceleration:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
  struct Location
  {
    double x;
    double y;

    Location(double x_arg,
             double y_arg)
             :
             x(x_arg),
             y(y_arg)
             {}
  };

  void accelerate()
  {
    velocity += acceleration;
  }
  void update_position()
  {
    x += velocity * cos(angle);
    y += velocity * sin(angle);
  }


Sorry, I'm a bit new to coding, so I often struggle going from concept to code. Here is my modified code from the suggestions I received:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
// main.cpp
#include "vehicle.h"

int main()
{
  veh::Vehicle car(4.9, 2.1, 100.0, 100.0, 0.0, 0.0);
  car.report();
  for (int i = 0; i < 3; i++)
    car.accelerate();
  for (int i = 0; i < 3; i++)
    car.brake();
  return 0;
}

// vehicle.h
#ifndef VEHICLE_H
#define VEHICLE_H

namespace veh
{
  struct Location
  {
    double x;
    double y;

    Location(double x_arg,
             double y_arg)
             :
             x(x_arg),
             y(y_arg)
             {}
  };

  struct Body
  {
    double width;
    double length;

    Body(double width_arg,
         double length_arg)
         :
         width(width_arg),
         length(length_arg)
         {}
  };

  class Vehicle
  {
    const Body vehicle_body;
    static const double speed_change = 1.0;

    Location position;
    double speed;
    double angle;

    void report_speed();

  public:
    Vehicle(double length_arg,
            double width_arg,
            double x_arg,
            double y_arg,
            double speed_arg,
            double angle_arg)
            :
            vehicle_body(length_arg, width_arg),
            position(x_arg, y_arg),
            speed(speed_arg),
            angle(angle_arg)
            {}
    void report();
    void accelerate();
    void brake();
  };
}

#endif

// vehicle.cpp
#include "vehicle.h"
#include <iostream>

namespace veh
{
  void Vehicle::report_speed()
  {
    if (speed <= 0.05 && speed >= -0.05)
      std::cout << "The vehicle is stopped.\n";
    else
      std::cout << "Vehicle's new speed: " << speed << " m/s\n";
  }

  void Vehicle::report()
  {
    std::cout << "Vehicle length: " << vehicle_body.length << " m\n"
              << "Vehicle width: " << vehicle_body.width << " m\n"
              << "Vehicle X Position: " << position.x << "\n"
              << "Vehicle Y Position: " << position.y << "\n";
    report_speed();
    std::cout << "Vehicle angle: " << angle << "\370\n"
              << "--------------------------\n";
  }

  void Vehicle::accelerate()
  {
    speed += speed_change;
    if (speed > 10.0)
      speed = 10.0;
    report_speed();
  }

  void Vehicle::brake()
  {
    speed -= speed_change;
    if (speed < 0.0)
      speed = 0.0;
    report_speed();
  }
}
That's a lot better than what was there in the beginning!

If you're interested in best practices, The C++ Core Guidelines, maintained by Bjarne Stroustrup and Herb Sutter, is probably as authoritative as it gets for C++.

They echo many of the things said on this thread, to point out the few I noticed:

"C.2: Use class if the class has an invariant; use struct if the data members can vary independently"
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-struct

"C.131: Avoid trivial getters and setters"
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rh-get

"C.49: Prefer initialization to assignment in constructors"
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-initialize


To point out a couple things not yet mentioned, this
1
2
3
4
5
6
   Vehicle(double length_arg,
            double width_arg,
            double x_arg,
            double y_arg,
            double speed_arg,
            double angle_arg)

is an example of poor interface design per
"I.4: Make interfaces precisely and strongly typed"
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Ri-typed

and this
void report();
is an example of poor class design per
"Con.2: By default, make member functions const"
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rconst-fct
Last edited on
has-a is when you put it in the class, its not 'true' inheritance at all.

eg

1
2
3
4
5
6
7
8
9
class mmft
{public:
    int x;
};
class meh
{
   vector<int> v; //meh has-a vector. 
   mmft m; //meh has-a mmft.   it LITERALLY has one of them, or it HAS A mmft in it. 
};


the more serious forms of inheritance are rabbit holes to be avoided until you NEED them. Has-a here is just being done for readability / organizational purposes, to break up something big into smaller bits.
meh j;
j.m.x; // its a little more organized, or it would be if I had typed meaningful names :P


Last edited on
@opisop

Good Work so far :+)

With relationships, there are 3 types, that I know of:

"IS A" implies inheritance, for example a Polygon IS A Shape. This ties in with the SOLID principle I mentioned earlier. It becomes important when you learn about virtual polymorphism. Basically the base class has an interface that applies to all the derived classes, for example Shape has area and perimeter functions.

"HAS A" The class has a member of a type. A Car has wheels, a Person has a Name (say std::vector<std::string>). This is also known as Composition or Aggregation, but there is a difference between the two:

https://en.wikipedia.org/wiki/Object_composition#Aggregation

"USES A" usually means a function has an argument of a particular type. A bicycle uses-a pump during a tire-pumping activity.

https://en.wikibooks.org/wiki/IB/Group_4/Computer_Science/Object-Oriented_Programming

Another thing about namespaces:

One can have a relatively long namespace name, then have a namespace alias to shorten it:

https://en.cppreference.com/w/cpp/language/namespace_alias


You can see from the example, that namespaces can be nested, so one would typically use them for something broader than a class, that is, the entire application and it's parts. So you might have a Transport namespace which has other things apart from Vehicle in it.

Some of the things I have mentioned might be too advanced for a beginner, I don't wish to overload you - most students don't have unlimited time to study everything. But I guess you could make a mental note that these things exist, and study them in depth later if you wish.

Keep up the good work :+D

Last edited on
Pages: 12