-1

I'm trying to learn c++, I've read a lot about it and get it, but every time I program something that uses OOP concepts I have memory problems.

This is the exeption I'm getting with my code:

First-chance exception at 0x002EFB60 in Mathmatician.exe: 0xC0000005: Access violation executing location 0x002EFB60.

If there is a handler for this exception, the program may be safely continued.

So my question here is: what is wrong with my specific code? And more importantly, how can I avoid such exeptions? Are ther rules of thumb in c++ to avoid these?

And another thing: How can I avoid returning a local variable from a function that gets deleted (I assume just return the value itself, not a pointer?)

(More info:This specific code will be used to calculate derivative, with diffrent formulas like devision, multiplication, and more inhereting from the Form virtual class)

// Mathmatician.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"
#include <typeinfo>
#include <iostream>
using namespace std;


#include <sstream>

template <typename T>
  std::string NumberToString ( T Number )
  {
     std::ostringstream ss;
     ss << Number;
     return ss.str();

  }

class Form {
public:
    virtual Form& Derive() = 0;
    
    Form& operator+(Form& const other); 
    Form& operator*(Form& const other); 
    virtual string Print() = 0;
};

class Number : public Form {
public:
    int a;
    Number(int a) : a(a)
    {}

     Form& Derive() 
    {
        return (Form&) Number(0);
    }
    
    string Print() 
    {
        return NumberToString(a);
    }
};

class Addition : public Form {
private:
     Form& a;
     Form& b;
public:
    Addition(Form& const a, Form& const b) : a(a),b(b)
    {
    }
    Form& Derive()
    {
        return a.Derive() + b.Derive();
    }
    
    string Print() 
    {
        return "("+a.Print();// + "+" + b.Print()+")";
    };
};

class Multiply : public Form {
private:
     Form& a;
     Form& b;
public:
    Multiply(Form& a, Form& b) : a(a),b(b)
    {
    }
    Form& Derive()
    {
        return *new Addition(a.Derive(),b.Derive());
        //return a.Derive()*b + b.Derive()*a;
    }
    
    string Print() 
    {
        return "("+a.Print() + "*" + b.Print()+")";
    };

};

inline Form& Form::operator+(Form& const other) // copy assignment
{
    return Addition(*this,other);
    //LocationNode* A = new LocationNode('A',1,2);
}
inline Form& Form::operator*(Form& const other) // copy assignment
{
    return Multiply(*this,other);
}

int _tmain(int argc, _TCHAR* argv[])
{
    Form* n = &(*new Addition((Form&)*new Number(5),(Form&)*new Number(5))).Derive();
    cout << (*n).Derive().Print() << "/n";

    return 0;


}

Thank you!

Community
  • 1
  • 1
Yanay Goor
  • 51
  • 9
  • 2
    What does your debugger say? – melpomene Apr 29 '18 at 08:15
  • 6
    Stop using `new` (ESPECIALLY `*new` ! ) and you should find memory problems magically disappear – M.M Apr 29 '18 at 08:16
  • `using namespace std;` <-- pls no (https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) – JHBonarius Apr 29 '18 at 08:16
  • 2
    `return a.ContainsX() || a.ContainsX();` What. – melpomene Apr 29 '18 at 08:16
  • 4
    `return (Form&) Number(0);` should fail to compile. I guess you are using MSVC with its crappy lvalue extension. The same mistake occurs in several places – M.M Apr 29 '18 at 08:17
  • 2
    `prog.cpp:25:27: error: 'const' qualifier may not be applied to a reference` – melpomene Apr 29 '18 at 08:18
  • @ melpomene It sould be `return a.ContainsX() || b.ContainsX();` but that has nothing to do with the problam, I'm not using that method yet. – Yanay Goor Apr 29 '18 at 08:18
  • 2
    `prog.cpp:38:16: error: C-style cast from rvalue to reference type 'Form &'` – melpomene Apr 29 '18 at 08:18
  • 2
    `prog.cpp:98:12: error: non-const lvalue reference to type 'Form' cannot bind to a temporary of type 'Addition'` – melpomene Apr 29 '18 at 08:18
  • 2
    `prog.cpp:103:12: error: non-const lvalue reference to type 'Form' cannot bind to a temporary of type 'Multiply'` – melpomene Apr 29 '18 at 08:19
  • 2
    Never saw such unreadable code for month! It is a "casting" show, full of new, obscure semantics and lots more. C++ is a different language! As a programmer you must be able to reduce your example code so that we have a chance to get to the details problem. Simply C&P all what you have here is not the way SO works! – Klaus Apr 29 '18 at 08:19
  • 2
    @YanayGoor If you're not using it, why did you post it? Please see [mcve]. – melpomene Apr 29 '18 at 08:20
  • @M.M Then how can I upcast properly? – Yanay Goor Apr 29 '18 at 08:20
  • 2
    @YanayGoor the problem is that you are returning references to objects that are destroyed when the function returns. The whole design is hopeless – M.M Apr 29 '18 at 08:21
  • What is your code supposed to do? It seems very simple algorithm, yet you try to do it in a very difficult way, making a lot of mistakes along the way. It's like trying to pick your nose, but putting on boxing gloves first. – JHBonarius Apr 29 '18 at 08:22
  • @Klaus I tried to do it how I would in c#, How should I have done that? – Yanay Goor Apr 29 '18 at 08:23
  • Even in C# you should make this so complex.... – JHBonarius Apr 29 '18 at 08:24
  • Regarding the `Number::Derive` function, if you want to return a polymorphic value that is local to the function (i.e. constructing the derived `Number` and returning it as the base `Form`) then you might use `unique_ptr`. So do something like `return std::make_unique(0)`. The function def should be altered accordingly to return `std::unique_ptr
    `. Alternatively, a shared_ptr. The differences between them can be found readily online. If you don't care about the difference, use `unique_ptr`.
    – iPherian Apr 29 '18 at 08:24
  • @YanayGoor Removing all `&` would be a good first step. – melpomene Apr 29 '18 at 08:24
  • @M.M I assumed that, but I cant identify where myself, and how to fix it. – Yanay Goor Apr 29 '18 at 08:25
  • @YanayGoor: No you did not! In C# you also would not use wild memory allocations ( simply impossible ). And obscure semantics are fully independent of the used language. Also assembler can be well formed and object orientated. But for a question on SO you have not to post everything and ask "Why it doesn't work". Start a debugger! But before, start reading a book for c++, objects, pro & cons for heap and stack, new and shared_ptr and so on. There is almost everything broken in your code. – Klaus Apr 29 '18 at 08:27
  • I suppose `shared_ptr
    ` would be one way to go about this , if you really want to keep the polymorphic hierarchy. (Personally I would consider polymorphism to be unnecessarily complicated this situation)
    – M.M Apr 29 '18 at 08:27
  • @iPherian it looks like the `operator+` and `operator*` have to store the inputs without destroying the original, so unique_ptr might not work – M.M Apr 29 '18 at 08:31

1 Answers1

-1

You may see my answer here. The problem of new is that it does not clear the memory of the allocated space. The solution for new is to add the following line as long as you initialized the elements.

memset(element,0,sizeof(element));

Therefore you can ensure there is no garbage in the memory.

This answer is somehow based on my C language experience. Recently I found that C++ has another function called std::fill

using namespace std;
/* some function */
        fill(elem,elem+sizeof(elem),0);
/* end of some funtion */
TravorLZH
  • 302
  • 1
  • 9
  • 1
    That advice is several flavours of misguided. Your constructors should properly initialize the class members. If you start `memset()`ing things, you're doing it wrong. – DevSolar Apr 29 '18 at 21:28
  • I am saying that you need to zero those initialized members – TravorLZH Apr 30 '18 at 01:59
  • And I am saying that `new` constructs the objects, that objects construct their members, and that using `memset` to zero those members is ill-advised. While the OP's code is terrible, he has initializers for all his class members, so your suggestion is not only ill-advised but also not the answer to his question. As for "learn C first", [don't do that](https://youtu.be/YnWhqhNdYyk), it is also bad advice. – DevSolar Apr 30 '18 at 04:42