CODEWITHSUNDEEP
c++pointers
user11607830
user11607830

Reputation:

Can anybody explain me why this code is not working?

I'm learning C++. Now, I'm trying to make one sample related with overloading operators of an object. My object (called Contador) has different methods and variables which help user to count iterations.

Header file of the object:

class Contador
{
private:
    int* Valor;
    int* Salto;

public:
    Contador(int Valor_Inicio = 0, int Salto = 1);
    ~Contador();

inline int Get_Valor() const { return *Valor; }
inline int Get_Salto() const { return *Salto; }

inline void Incremento() { Set_Valor(Get_Valor() + Get_Salto()); }

inline void operator++ () { Set_Valor(Get_Valor() + Get_Salto()); }

void Set_Valor(int Valor);
void Set_Salto(int Salto);

};

Cpp file of the object:

// Librerias Propias
#include "Contador.h"

Contador::Contador(int Valor_Inicio, int Salto)
{
    Set_Valor(Valor_Inicio);
    Set_Salto(Salto);
}

Contador::~Contador()
{
    delete Contador::Valor;
    delete Contador::Salto;
}

void Contador::Set_Valor(int Valor)
{
    delete Contador::Valor;
    Contador::Valor = new int(Valor);
}

void Contador::Set_Salto(int Salto)
{
    delete Contador::Salto;
    Contador::Salto = new int(Salto);
}

The main() function of the sample has 2 different for loops. In the first one, I call Incremento() method and in the second one I call the overloaded operator.

Main function:

void main()
{
    // Genero el elemento de analisis.
    Contador* contador = new Contador();

    // Realizo el bucle con la función de incremento.
    std::cout << "Incremento()" << std::endl;
    for (contador->Set_Valor(0); contador->Get_Valor() < 3; contador->Incremento())
    {
        // Escribo algo.
        std::cout << "Iteracion actual: " << contador->Get_Valor() << std::endl;
    }

    // Realizo el bucle on el operador sobrecargado
    std::cout << "operador sobrecargado" << std::endl;
    for (contador->Set_Valor(0); contador->Get_Valor() < 3; contador++)
    {
        // Escribo algo.
        std::cout << "Iteracion actual: " << contador->Get_Valor() << std::endl;
    }
}

The problem appears when main function passes the first iteration of the second loop. It throws one exception in Get_Valor() method.

Exception image

It seems to me that it change the memory addres of the pointer Valorin some place, but I can`t find where.

Can anybody help me? Thanks.

Upvotes: 0

Views: 121

Answers (4)

Boris Lyubimov
Boris Lyubimov

Reputation: 201

In addition to previous answers. As I could see Contador* contador = new Contador(); code also contains UB (undefined behaviour) This call is equal to constructor with parameters Contador(0, 1) which will do Set_Valor and Set_Salto which call delete first but at this moment content of this variables is not guaranteed to be nullptr so you might corrupt data. Also compiler if it sees UB might optimize out all other code since it's already UB and it can change behaviour anyway it wants for example throw it away completely. https://devblogs.microsoft.com/oldnewthing/20140627-00/?p=633

Upvotes: 0

Jarod42
Jarod42

Reputation: 217085

3 coding issues:

  • main shoudl return int.

  • Valor and Salto are not initialized in constructor. Contador::Set_Valor and Contador::Set_Salto requires initialized pointers (as you delete them).

    Easy fix is:

    class Contador
{
private:
    int* Valor = nullptr;
    int* Salto = nullptr;
//...
};

  • Last issue is in your last loop:

    for (contador->Set_Valor(0); contador->Get_Valor() < 3; contador++)

    As condator is a pointer (not pointing on an array), accessing condator[1] would be UB. You wanted ++(*condator) (operator++ () is pre-increment whereas operator++ (int) is post-increment).

Finally, avoiding usage of all those pointers would simplify code (and no bother with rule of 3 you break):

class Contador
{
private:
    int Valor;
    int Salto;

public:
    Contador(int Valor = 0, int Salto = 1) : Valor(Valor), Salto(Salto) {}
    ~Contador() = default;

    int Get_Valor() const { return Valor; }
    int Get_Salto() const { return Salto; }

    void Incremento() { Set_Valor(Get_Valor() + Get_Salto()); }
    void operator++ () { Set_Valor(Get_Valor() + Get_Salto()); }

    void Set_Valor(int Valor) { this->Valor = Valor;}
    void Set_Salto(int Salto) { this->Salto = Salto;}
};

int main()
{
    Contador contador;

    std::cout << "Incremento()" << std::endl;
    for (contador.Set_Valor(0); contador.Get_Valor() < 3; contador.Incremento())
    {
        std::cout << "Iteracion actual: " << contador.Get_Valor() << std::endl;
    }

    std::cout << "operador sobrecargado" << std::endl;
    for (contador.Set_Valor(0); contador.Get_Valor() < 3; ++contador)
    {
        std::cout << "Iteracion actual: " << contador.Get_Valor() << std::endl;
    }
}

Upvotes: 2

molbdnilo
molbdnilo

Reputation: 66371

contador++ does not do what you think it does - contador is a pointer, not a Contador, so it will make contador point to something that does not exist.
You need to dereference the pointer.

However, *contador++ also increments contador - it is *(contador++) - and (*contador)++ does not compile because you have only overloaded the prefix operator (the postfix operator has the prototype operator++(int).

So, ++*contador will do what you want.

You can avoid many similar problems, and the clunky syntax, by not using pointers unnecessarily.

Upvotes: 3

Adrian Mole
Adrian Mole

Reputation: 51815

The expression contador++ increments the address that contador (a pointer) points to! So, after the first iteration, the pointer will be completely invalid.

To call the increment operator, you need: ++(*contador) which first dereferences the pointer to the object pointed to, then effects that object's increment operator.

Upvotes: 2

Related Questions