Pass parameter on constructor or pass them on each method

Question

I'm learning C++ and I have this header file:

#ifndef _ROBOT_MOVE_H__
#define _ROBOT_MOVE_H__ 1

#include "ros/ros.h"

class RobotMove
{
private:
    double current_linear;
    double current_angular;
public:
    void TurnLeft(const ros::Publisher& publisher, ros::Rate& loop_rate);
    void TurnRight(const ros::Publisher& publisher, ros::Rate& loop_rate);
    void TurnAround(const ros::Publisher& publisher, ros::Rate& loop_rate);
    void Forward(const ros::Publisher& publisher, ros::Rate& loop_rate);
    void Backward(const ros::Publisher& publisher, ros::Rate& loop_rate);
    void Stop();

    RobotMove();
    ~RobotMove();
}


#endif

I don't know if it is better to pass const ros::Publisher& publisher, ros::Rate& loop_rate once, on the constructor, or pass them every time I call the other methods.

Is it better to pass these two parameters in constructor and store a pointer in the class or pass them on each method?

By the way, the two parameter will be always the same objects.

Without knowing the context I would say neither. Pass them to the constructor and store them as **objects** in `RobotMove`. — john, Jul 18 '19 at 17:37
`#define _ROBOT_MOVE_H__` That's a reserved identifier. By defining it, the program will have undefined behaviour. — eerorika, Jul 18 '19 at 17:42
Will you ever run into a situation where a single instance of an object will use a different publisher? Best to pass it in each time. Generally going to have the same publisher each time, you might want to consider storing a publisher instance and use that instead. If you are making changes to the publisher between calls, you may need to store a reference to the publisher. — , Jul 18 '19 at 17:44
Also, this has a high chance of getting purely opinionated answers. — , Jul 18 '19 at 17:44
The job of a constructor is to create a functional object and establish/satisfy class invariants. If passing the argument is needed to do that it should be a constructor argument. If it is not essential and the object can be used without, then it *may* be passed to functions later. — Jesper Juhl, Jul 18 '19 at 17:47
`#ifndef _ROBOT_MOVE_H__` - That's a *very bad* name for your header guard. It falls to two rules of the language: 1) Any name starting with underscore followed by a uppercase letter is reserved for the implementation. 2) any name containing double underscore *anywhere* is reserved for the implementation. You are *not* permitted to use such names in your own code and doing so causes your code to have Undefined Behaviour and the entire program is essentially invalid. — Jesper Juhl, Jul 18 '19 at 17:53
@JesperJuhl Ok, show me how you will do it. As I said at the first line of my question: "I'm learning". Any sample is welcome. — VansFannel, Jul 18 '19 at 17:57
@VansFannel "show me how you will do it" - Are we talking about the header guard here? If so, I'd do `#define MY_PROJECTNAME_MY_CODEMODULE_MY_FILENAME_MY_RANDOM_ID_THAT_NOONE_ELSE_USES` - For example (from an actual project of mine): `#define ROLLERCUBE_UI_MOTION_ROLLBALL_H_42666`. Doesn't trample on any standard rules about reserved names and is unique enough to not clash with any library I may be using. — Jesper Juhl, Jul 18 '19 at 18:02

R Sahu · Accepted Answer · 2019-07-18T18:02:54.577

5

By the way, the two parameter will be always the same objects.

To me, that's an indication that they should be member variables of the class. Making them member variables of the class follows the Don't Repeat Yourself (DRY) principle. Hence, it makes sense to pass them in the constructor and let the object store them as member variables.

edited Jul 18 '19 at 18:02

answered Jul 18 '19 at 17:45

R Sahu

204,454
14
159
270

Thanks a lot for your answer. Maybe I can store a reference to those two variables. I'm new in C++ and I don't know how to do it. With a reference, I mean not create a copy object or a new instance. Thanks. – VansFannel Jul 19 '19 at 13:30
1

You sure can. Make the member variables reference types and make sure to initialize them in the constructor. Ex: `struct Foo { int& ref; Foo(int& in) : ref(in) {} };` – R Sahu Jul 19 '19 at 15:37

score 4 · Answer 2 · answered Jul 18 '19 at 17:51

This is a good fundamental question.

To answer it you can ask yourself are these parameters specific to each action(method) or are they objects used by an instance of the class in general ?

In your example given the name of your class and your methods, the fact that so many methods uses them and that you additionally specified that they don't change, I would suggest passing them to the constructor rather than on each call.

This won't always be the correct answer though, you have to judge and balance based on how often you think these parameters might change but also on what semantical sense you want to give to your class ans its relationship toward these parameters.

I know it sounds like a lot of fluff, but as often design choices are context dependent.

Thanks for your answer. These objects will never change. – VansFannel Jul 18 '19 at 17:55 — VansFannel, Jul 18 '19 at 17:55

StormsEngineering · Answer 3 · 2019-07-19T21:10:38.887

3

I would say that from a programming perspective, the less you have to copy and paste the same code over and over again the better. That is the point of programming make the computer repeat things for you. I would pass those parameters to the constructor and then just store them as properties. I believe you would implement it like below:

class RobotMove
{
private:
    double current_linear;
    double current_angular;
    const ros::Publisher * publisher;
    ros::Rate * loop_rate;

public:
    RobotMove(const ros::Publisher& publisher, ros::Rate& loop_rate)
    {
        this->publisher = &publisher;
        this->loop_rate = &loop_rate;
    }
};

edited Jul 19 '19 at 21:10

answered Jul 18 '19 at 17:44

StormsEngineering

656
1
6
18

Keep in mind that "reducing copying and pasting" in and of itself isn't a great reason to make structure changes like this. Sometimes doing this can result in oversimplification and confusion. Usability should be a major factor in these types of decisions. – Jul 18 '19 at 17:51
@FrançoisAndrieux Does my edit above illustrate a little better? – StormsEngineering Jul 18 '19 at 17:54
@Chipster This would remove having to pass those two parameters everytime, and instead just do it once in the constructor correct? I feel that would a be a better design? – StormsEngineering Jul 18 '19 at 17:55
@StormsEngineering I agree, that would be better design. I'm just saying that the reason you give isn't necessarily a good one *by itself*. In this case, there are other reasons that also point to it needing to get moved to a constructor, so in the end, you are right. I'm just disagreeing with the reason you are giving for it. – Jul 18 '19 at 17:58
@Chipster Right, that makes perfect sense. Thanks for the clarification! – StormsEngineering Jul 18 '19 at 18:03
@StormsEngineering Not quite, the constructor still does nothing with the constructor's arguments. Additionally, by introducing reference data members, you introduce [new complexities and restrictions](https://stackoverflow.com/questions/892133/should-i-prefer-pointers-or-references-in-member-data) that simpler pointer members wouldn't have. – François Andrieux Jul 18 '19 at 18:08
@FrançoisAndrieux I updated my answer again. I have learned a lot about this too, hahha. That should assign the addresses of the parameters to the pointer properties correct? – StormsEngineering Jul 19 '19 at 21:15
@StormsEngineering That seems fine. But you should read about [member initializer lists](https://en.cppreference.com/w/cpp/language/initializer_list). It won't make a difference here, but it's a good habit to get into. – François Andrieux Jul 20 '19 at 22:46

Pass parameter on constructor or pass them on each method

3 Answers3