0

When I started working with C++, I put everything into one big .cpp file.

I'm now trying to change this by creating classes for my functions. But I encountered a problem.

I have a function where there's an index which is supposed to get raised by 1 under certain circumstances & keep this value for the next time. This index is used for renaming a file for example. Like this:

// if there's "motion" create new video file, reset index

if (numCont > 0 && contours.size() < 100) {
    index = 0;
    MotionDetector::saveFile(frame1, output_file, addFrames, video_nr);
}

// if there's a video file & no motion, raise index by 1
// if there's been no motion for x frames (that's what the index is for),
// stop recording

if (addFrames && numCont == 0) {
    index++;
    MotionDetector::saveFile(frame1, output_file, addFrames, video_nr);
    if(index == 30) addFrames = false;
}

Inside of saveFile is something like this:

if (!addFrames) {
    // release old video file
    // create new file using video_nr for the naming part
}
else {
    //add frame to existing video
    video << frame;
} 

Another variable I want to change "globally" is a boolean. If certain conditions are met it is either true or false and this state has to be saved for the next time the function gets called.

I know that I could return the values and then use it from my main again BUT then I'd have ~3 different variables (one Mat, one int, one bool) to return and I have no idea how to handle that.

Before I moved it to its own class I still had 2 variables to make it work, even though I kinda got to the point of thinking it was quite useless that way. Is there any easy way to keep changes made by functions in external classes? Tried it using reference already, but it didn't seem to work either.

Right now addFrames is a referenced argument passed from the original class/function-call but it doesn't work as intended.

Appuru
  • 417
  • 1
  • 6
  • 16
  • 1
    Rewrite it all after having read books about how to write good code. I can only guess, but my spider sense tells me you have an unmaintainable piece of software there, with about a ton of bugs. – Sebastian Mach Oct 01 '14 at 14:08
  • 1
    Unclear what you are asking. I guess member-variables or static storage be important somehow. Who could know... – Deduplicator Oct 01 '14 at 14:09
  • 1
    Do you want a static variable inside the function? – Neil Kirk Oct 01 '14 at 14:11
  • 1
    @NeilKirk: That only fixes the symptom, but not the cause (which is bad code). I vote for "Too broad". – Sebastian Mach Oct 01 '14 at 14:13
  • Your question is not clear. Explain what you want to accomplish, show us the code you've written to accomplish it, and then we can help you see why it doesn't work or why it is undesirable (and provide a more elegant solution). – derpface Oct 01 '14 at 14:18
  • @phresnel It's fine for getting a unique id for something. – Neil Kirk Oct 01 '14 at 14:20
  • @NeilKirk: No, it's not. I can enumerate more cases where this is bad for unique ids than I can imagine good applications thereof. – Sebastian Mach Oct 01 '14 at 14:22
  • @phresnel I seldom need more than 18,446,744,073,709,551,615 ids per run of my program. – Neil Kirk Oct 01 '14 at 14:46
  • @NeilKirk: I am not sure what a specific value-range has to do with static variables not being a good solution to id-generation. – Sebastian Mach Oct 01 '14 at 14:52
  • @phresnel Ok I bite what is wrong with static variable as a source of unique arbitrary id. – Neil Kirk Oct 01 '14 at 14:58
  • @NeilKirk: There are many arguments against using static variables in general; basically the same arguments that also go against singletons and global variables (singletons and static function variables are just instances of global variables). That was my initial point. Keywords are: Multithreading, Global State, Reusability. You cannot simply do "GameObject g {generate_id();}" and "GraphicsObject fx {generate_id();}" and expect it to work as soon as you separate graphics and game initialization into just two threads. Why not a separate generator object for each domain? – Sebastian Mach Oct 01 '14 at 15:10
  • This goes too far for comments, so I cut down to just what I found most important: Debugging. It's hard or impossible to reproduce minimal testcases in multithreaded environment when there's no control of who calls certain functions, and when there's no clear dependency between call-tree and state-tree, whereas with a non-global generator object, the object passing chain is well defined. This argument is even true for single-threaded programs. Genericity and testability are also good reasons for why C++ now has RNG classes, finally doing away with `std::rand()`. – Sebastian Mach Oct 01 '14 at 15:13
  • @phresnel Ok fair point, it is not a scalable use. But I think it's ok for simple, single-threaded use. – Neil Kirk Oct 01 '14 at 15:32
  • @NeilKirk: That, yes. But then, it's just 7 vs. 4 lines of code for an implementation that is not thread-safe in itself, and does not interfere with a "avoid global state" policy. – Sebastian Mach Oct 01 '14 at 15:59
  • well, i guess this got out of hand while I was gone. I actually wanted to add further specifics to the question as I was on the run while writing it so I wouldn't forgot to ask after all ^^ I'll edit the question right now and hope it'll make it more obvious. – Appuru Oct 01 '14 at 17:02

1 Answers1

1

You shouldn't need global state for this, if you consider your code doing motion detection and maintaining state to be wrapped up into a class. You may even derive it from MotionDetector, I don't know what your problem is exactly:

class MyMotionDetector : public MotionDetector {
private:
    bool addFrames;
    int index;
    Mat mat;
public:
    MyMotionDetector () :
        MotionDetector (/* ... */), addFrames (false), index (0), mat ()
    {
        /* constructor code */
    }

    void processNewData (Contours & contours, /* whatever */) {
        /* whatever */
        if (numCont > 0 && contours.size() < 100) {
            index = 0;
            MotionDetector::saveFile(frame1, output_file, addFrames, video_nr);
        }
        /* whatever */
    }

    /* whatever methods */

    ~MyMotionDetector () :
    {
        /* destructor code */
    }
};

Before you think about using globals, consider a question like "why might I not have two of these in the same program, creating files in two different directories?" Designing your objects properly can mean that things like counters and state booleans are kept close to the code that needs to manage it. And you have more flexibility to instantiate more than one of them.

As for returning more than one value from a function, the magic phrase you're looking for would be "multiple return values". There are a few ways to do it:

Returning multiple values from a C++ function

Community
  • 1
  • 1
  • Thanks for editing my question and for the answer ofc. I'm still pretty new to the whole C++ stuff and was more concentrated on WHAT the code should do than on HOW it does it so far :D that's why I haven't looked into header files and classes as such too much before. That's pretty much the reason I got stuck there and asked for help before using another few hours experimenting on it. I'll look into it later on and hopefully get more used on how to go about such problems. – Appuru Oct 01 '14 at 18:22
  • 1
    @Appuru If you find yourself in a situation with code that works but makes you uneasy for some reason in the design, consider [codereview.stackexchange.com](http://codereview.stackexchange.com) – HostileFork says dont trust SE Oct 01 '14 at 18:23
  • Thanks for linking that, I might be using it in the future. I forgot to initialize my MotionDetector once in the main and called the function through MotionDetector().detectMotion(...) in my while loop.. so it got initialized again and again. Silly me. /e: like this it even works the way I had it before (just set the variable values in my header - any reason not to do so?).. so it just was "MotionDetector motion;" missing in my main... – Appuru Oct 02 '14 at 10:06