Iterating over pointer array C++

I am new to C++ but I have an experience in C. Now I am trying to iterate over an array of objects but I am getting segmentation fault because it runs out of the range of the array.

I have a class ZombieHorede which keeps a pointer to an array of zombies.

#pragma once

#include "Zombie.hpp"

class ZombieHorde {
private:
        static std::string      namepool[7];
        static std::string      typepool[7];
        Zombie                          *zombies;
public:
        ZombieHorde(int N);
        ~ZombieHorde(void);

        void    announce(void) const;
};

The constructor of this class takes a number N and initializes the array of N zombies. Like this.

ZombieHorde::ZombieHorde(int N)
{
        int             i;

        i = 0;
        this->zombies = new Zombie[N];
        while (i < N)
        {
                this->zombies[i].set_name(this->namepool[rand() % 7]);
                this->zombies[i].set_type(this->typepool[rand() % 7]);
                i++;
        }
}

Now I want to iterate over zombies and call a function on each of them ... like this...

void    ZombieHorde::announce() const
{
        int     i;

        i = 0;
        while (&this->zombies[i])
        {
                this->zombies[i].announce();
                i++;
        }
}

And the announce function gives segmentation fault cuz i goes beyond the range of the array with this main:

#include "ZombieHorde.hpp"

int     main()
{
        ZombieHorde     horde(4);

        horde.announce();
}

Now the main question is how to iterate over the zombies array in order to not get a segmentation fault.

ATTENTION: Note that this is a 42 school exercise and I am not allowed to use STL of C++. I should allocate zombies at once in the constructor.

Solution 1:

As i read the given headline to your question , I thought you really have an "pointer array" and iterate over it. But you coded an array of objects, which is a different thing. OK, so lets start creating a real pointer array...

Your problem is while (&this->zombies[i]). This will test if the result is true or false, if you have pointers, they will be implicit converted to bool, as this we expect a nullptr at the end of the array.

Maybe you change the allocation and initializing of your array to:

    Zombie **zombies = new Zombie*[N+1];

    for ( unsigned int idx=0; idx<N; idx++)
    {    
        zombies[idx] = new Zombie;
    }    
    zombies[N] = nullptr; // important to have the "end" mark if we later test for nullptr!

    // use it later maybe per index
    for ( unsigned int idx=0; idx<N; idx++) 
    {    
        zombies[idx]->announce();
    }    

    // or as you tried with pointer to nullptr compare
    // you can also use your While instead if you initialize
    // your running variable before outside the loop
    for ( Zombie** ptr= zombies; *ptr; ptr++)
    {    
        std::cout << ptr << std::endl;
        (*ptr)->announce();
    }    

BTW: No need to write this in this context!

But you should start to write C++ instead of "C with classes"!

class Zombie
{
    private:
        std::string name;
        std::string type;

    public:
        // you should always use constructors to set content
        // of the class at the beginning instead later overwrite
        // values. Making things private and write accessor's to
        // all and everything is not the use case of encapsulation
        Zombie( const std::string& name_, const std::string& type_): name{name_},type{type_}{}
        void announce() const { std::cout << "Zombie " << name << ":" << type << std::endl; }
};

class ZombieHorde {
private:
        static std::string namepool[7];
        static std::string typepool[7];
        std::vector<Zombie> zombies;

public:
        ZombieHorde(const unsigned int N);
        ~ZombieHorde(){}

        void announce() const;
};

std::string ZombieHorde::namepool[7] = { "One","two","three","four","five","six","seven"};
std::string ZombieHorde::typepool[7] = { "red","blue","green","yellow","pink","mouve","black"};


ZombieHorde::ZombieHorde(const unsigned int N)
{
    for ( unsigned int idx=0; idx < N; idx++ )
    {    
        zombies.emplace_back( this->namepool[rand() % 7], this->typepool[rand() % 7]); 
    }    
}

void ZombieHorde::announce() const
{
    // iterate over containers is now simplified to:
    for ( const auto& zombie: zombies ) { zombie.announce(); }
}

int main()
{   
    ZombieHorde  horde(4);
    horde.announce();
}

Solution 2:

question is how to iterate over the zombies array in order to not get a segmentation fault.

One way to resolve this would be to have a data member called zombieSize that stores the value of N as shown below in the modified snippets:

class ZombieHorde {
private:
        static std::string      namepool[7];
        static std::string      typepool[7];
        Zombie                          *zombies;
        std::size_t zombieSize; //data member that is used to store the value of N
public:
        ZombieHorde(int N);
        ~ZombieHorde(void);

        void    announce(void) const;
};

//use member initializer list to initialize zombieSize
ZombieHorde::ZombieHorde(int N): zombieSize(N)
{
        int             i;

        i = 0;
        this->zombies = new Zombie[zombieSize];//use data member zombieSize instead of N
        
        while (i < zombieSize)//use zombieSize instead of N
        {
                this->zombies[i].set_name(this->namepool[rand() % 7]);
                this->zombies[i].set_type(this->typepool[rand() % 7]);
                i++;
        }
        
}
void    ZombieHorde::announce() const
{
        int     i;

        i = 0;
        while (i < zombieSize)//use zombieSize instead of N
        {
                this->zombies[i].announce();
                i++;
        }
}