-1

I am having a problem with assigning new values to a dynamic int array that is a data member variable of the class IntersectionFlowRate(). I can initialize and print the values of the array inside the constructor. However, when I exit the constructor to the another class and then later call a function within the IntersectionFlowRate() class passing in variables to overwrite the initial values of the data member it will segmentation fault. I have debugged to find that overwriting the array is causing the seg fault. And that even attempting to access the dynamic array within one of its functions will seg fault.

My question is how can I edit the values of a dynamic int array member variable from within one of its functions i.e setArrayElement(int index, int x).

Here is some of my code. Sorry if I am unclear or missing something ridiculous. I have been stuck on this for hours.

    #ifndef INTERSECTIONFLOWRATE_H
    #define INTERSECTIONFLOWRATE_H

    class IntersectionFlowRate
    {
    public:
        IntersectionFlowRate();
        ~IntersectionFlowRate();
        void setFlowCycle(int index, int flow);

    private:
        int* m_flowRateMotorCycle;

 };
 #endif

in the .h file ^

    #include "IntersectionFlowRate.h"
    #include <cstdlib>
    #include <iostream>
    #include <new>

    using namespace std;
    IntersectionFlowRate::IntersectionFlowRate()
    {
        const int SIZE = 4; //Constant for m_flowRates[] size

        //DYNAMIC MEMORY DELETE LATER
        m_flowRateMotorCycle = new int[SIZE];

        for(int i = 0; i < SIZE; i++){
            m_flowRateMotorCycle[i] = 0;
            cout << m_flowRateMotorCycle[i] << endl;
            cout << "WE GOT HERE" << endl;
        }
    }

    void IntersectionFlowRate::setFlowCycle(int index, int flow){
        cout << "INDEX: " << index << endl;
        cout << "FLOW: " << flow << endl;

        m_flowRateMotorCycle[index] = flow; //seg fault is here
    }

I have another class that creates a pointer to a IntersectionFlowRate() object and then calls its setFlowCycle function passing in two VALID ints. With the debugging I was able pass 0 and 3 to the function setFlowCycle(0, 3) just fine and output those variables within the function.

    #ifndef TRAFFICSIM_H
    #define TRAFFICSIM_H

    #include "IntersectionFlowRate.h"

    using namespace std;

    class TrafficSim
    {
    public:
        TrafficSim(); //Default Constructor
        TrafficSim(const char* file); //Constructor
        ~TrafficSim(); //Destructor


    private:
        IntersectionFlowRate* m_flowRate;
    };
    #endif


    #include "TrafficSim.h"
    #include "IntersectionFlowRate.h"
    #include <iostream>
    #include <string>
    #include <fstream>
    #include <cstdlib>


    using namespace std;

    TrafficSim::TrafficSim()
    {

        IntersectionFlowRate* m_flowRate = new IntersectionFlowRate();
        m_flowRate->setFlowCycle(0, 3);

    }

I replicated the error with this code. If no one else can I am completely unsure of what is possibly wrong anymore.

Toasty
  • 49
  • 6
  • 2
    You should provide [MCVE](http://stackoverflow.com/help/mcve). – MikeCAT Oct 01 '15 at 01:16
  • A [minimal](http://stackoverflow.com/help/mcve) example is not the code where you *think* the issue is, it's the smallest possible **complete** code that replicates the issue. My guess is you have a [rule of 0/3/5](http://en.cppreference.com/w/cpp/language/rule_of_three) violation. Post a complete example. – user657267 Oct 01 '15 at 01:17
  • [A simple test](http://melpon.org/wandbox/permlink/zZktmhgtuwPpZVPf) didn't give me Segmentation Fault. Did you define (and initialize it if needed) `m_flowRateCar` properly? – MikeCAT Oct 01 '15 at 01:19
  • 2
    In real life you would use `vector` instead of `int *`, I assume you are doing it this way as a learning exercise? – M.M Oct 01 '15 at 01:20
  • @Toasty How is this complete? We still can't copy and compile this code. – user657267 Oct 01 '15 at 01:28
  • EDIT: Okay I updated the code more the I defined and initialized the other arrays the same way I just left them out originally for readability. I would use vector normally and I guess I suppose I still could. I didn't check over the requirements to see if it explicity said not to use them.. How would I implement them into this? – Toasty Oct 01 '15 at 01:30
  • What's `NORTH`? If it isn't 0, 1, 2, or 3, that could be your problem. – 1201ProgramAlarm Oct 01 '15 at 01:30
  • @Toasty Please read the mvce link that's been posted twice above, nobody can tell you what is wrong with your program until you provide a complete example. – user657267 Oct 01 '15 at 01:32
  • @user657267 I have and still am. I posted this less than 22 minutes ago it takes a little longer than a couple minutes to read that link and then go and edit my code thanks. – Toasty Oct 01 '15 at 01:37
  • @Toasty You might want to delay editing your question until you have a complete example, or even delete and repost when you're ready to reset the down votes. – user657267 Oct 01 '15 at 01:39
  • @Toasty C++ is not Java. You're even calling `new` for things that are totally unnecessary, like this: `IntersectionFlowRate* m_flowRate = new IntersectionFlowRate();` Instead, do this: `IntersectionFlowRate m_flowRate;` In addition, using `std::vector` would (more than likely) get rid of most, if not all of your runtime issues. – PaulMcKenzie Oct 01 '15 at 02:03
  • @PaulMckenzie Taking out that new got me past my current point. I knew I was confusing something with something else. Thanks! – Toasty Oct 01 '15 at 02:13
  • Pointers are like fire. Don't fear them, but do respect the damage they can do when improperly controlled. – user4581301 Oct 01 '15 at 02:14
  • @Toasty See my answer. It is not the real problem. – PaulMcKenzie Oct 01 '15 at 02:18
  • Please use an online C++ IDE to ensure you give complete examples. You didn't include a main(), so you did *not* give a program that others could use to reproduce the problem. [Here is a compilable example](http://coliru.stacked-crooked.com/a/baef0124142f8b33)...but it's not complete because it doesn't reproduce a problem. In the future make sure you can demo the problem working online before asking. Use [coliru-stacked-crooked.com](http://coliru.stacked-crooked.com/) or [ideone](http://ideone.com) or [http://cpp.sh](http://cpp.sh/) or whatever. Pare down, remove headers, simplify. – HostileFork says dont trust SE Oct 01 '15 at 02:28
  • 1
    I had no idea online C++ IDE's even existed. I will definitely make use of that next time. Thank you. – Toasty Oct 01 '15 at 02:50

2 Answers2

2

You are setting a local variable called m_flowRate, not the member variable m_flowRate of your TrafficSim class:

Instead of this:

TrafficSim::TrafficSim()
{
    IntersectionFlowRate* m_flowRate = new IntersectionFlowRate();
    m_flowRate->setFlowCycle(0, 3);
}

It should be this:

 TrafficSim::TrafficSim()
 {
    m_flowRate = new IntersectionFlowRate();
    m_flowRate->setFlowCycle(0, 3);
 }

But overall, it not need be a pointer. It could be an object member within your class. That would cut down on the pointer usage a bit:

class TrafficSim
{
    public:
        TrafficSim(); //Default Constructor
        TrafficSim(const char* file); //Constructor

    private:
        IntersectionFlowRate m_flowRate;
};

Then:

TrafficSim::TrafficSim()
{
    m_flowRate.setFlowCycle(0, 3);
}

As to your question as to how to incorporate usage of std::vector in your class, here is a code sample of the IntersectionFlowRate class, rewritten using vector:

Vector sample

Also, another source of problems is that your classes fail to follow the Rule of 3 when you have pointers to dynamically allocated memory in your class.

Using std::vector takes care of this automatically, but if you insist on using pointers, you need to adhere to the directions at the link posted.

Community
  • 1
  • 1
PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • Awesome this fixed it. Thank you so much. Ironically I read an answer for this hours ago, but I'm so stressed out I must've missed it. – Toasty Oct 01 '15 at 02:24
0

Yes, use a std::vector, it is much simpler, and it is a template so it also pretty fast and works any type (best for primitive types or pointers to objects) , and it also has boundary checking and other useful things.

If you need fast array-like access then you could use std::map which associates a key with a value, like so

    std::map<UINT, YourClass*> m_mapIDs_to_YourClass;

When you first start using stl containers they might seem a little strange, but after a short while you cannot do without them, luckily they have been part of the C++ standard for some time now.

Boundary check for both these containers can be done by comparing your iterator to mapYourMap.end(), if they are equal you have passed the last element and trying to access data through the iterator will cause an exception. Example for std::vector (if vecInt is a vector< int >):

    vector<int>::iterator it = vecInt.begind();
    if (it == vecInt.end()) return; // vector is empty
    do {  // runs through elememts until out of bound, useful for searching
        i++
    while (it != vecInt.end());
phazer
  • 89
  • 7
  • Recommend expanding the bit on boundary checking because the vast majority of vector calls do no bounds checking. [The one exception I know of is `at`](http://en.cppreference.com/w/cpp/container/vector/at). – user4581301 Oct 01 '15 at 03:19
  • No, not automatically, that would be wasteful but you can do this vector::iterator it = vecType.begin(); it++; if (it == vecType.end()) ; // passed last element – phazer Oct 01 '15 at 03:25
  • I added the info, is it clear enough or should I add example code? – phazer Oct 01 '15 at 03:29
  • I meant that you CAN do boundary checks, you cannot do that on a simple array. If the calls automatically did boundary checks it would be very hard to optimize stuff like search and sort algorithms – phazer Oct 01 '15 at 03:35
  • std::begin and std::end extend iterator support to static arrays. Dynamic... Well, yer on yer own, kid. – user4581301 Oct 01 '15 at 03:44
  • Ah, yes static arrays but what are those useful for besides string constants and very low level code or in assembly, and then I always store pointers, never values, then I can pretend that the static array is sort of dynamic by filling it with NULL pointers and insert NULL when I delete/remove an entry, then my code looks for an entry with a NULL pointer and inserts the new entry there. That works well for some stuff, and it is the fastest possible linear container, I would never dream of using stl containers in assembly :) (I am not even quite sure how I would do templates in assembly :) ) – phazer Oct 01 '15 at 04:10
  • Very different philosophies at play here. I almost never use pointers, and when I do they tend to be from a statically allocated pool to ensure a predictable lifetime and locality and to minimize fragmentation. I probably spend more time optimizing memory accesses than I do code. Arrays of pointers typically mean you're accessing randomly throughout memory and thrashing the cache. Once that starts it doesn't matter how efficient the code is, your performance is gated at the RAM. – user4581301 Oct 01 '15 at 05:42