-2

I'm having trouble with this piece of code which shall evaluate numeric operations. I tried searching, but no answer seemed to be helpful.

The base class to represent an operation is:

Operacija.h:

#pragma once
#include <iostream>
using namespace std;

class Operacija
{
protected:
    char* naziv;
    int drugiO;
    Operacija* suprotna;
public:
    Operacija* retOp() { return this->suprotna; }
    int retD() { return this->drugiO; }
    void printOp()
    {
        cout << "Ime operacije: " << this->naziv << endl;
        cout << "Drugi operand: " << this->drugiO << endl;
        cout << "Ime suprotne operacije: " << this->suprotna->naziv << endl;
    }
    virtual int DoOperation(int op1, int op2) = 0;
};

This derived class is for multiplication:

Mnozenje.h:

//this is .h for 1st class
#pragma once
#include "Operacija.h"
#include "Deljenje.h"

class Mnozenje : public Operacija
{
public:
    Mnozenje(int b);
    int DoOperation(int op1, int op2);
};

Mnozenje.cpp:

#include "Deljenje.h"
#include "Mnozenje.h"
class Deljenje;
Mnozenje::Mnozenje(int b)
{
    Deljenje* a = new Deljenje(b);
    naziv = "Mnozenje";
    suprotna = a;
    if (b == 0)
    {
        cout << "operand ne sme biti 0, stoga je stavljen na ";
        cout << "default tj. postavljen na vrednost 1!" << endl;
        drugiO = 1;
    }
    else
        drugiO = b;
}

int Mnozenje::DoOperation(int op1, int op2)
{
    int a = 0;
    a = op1 * op2;
    return a;
}

And this derived class is for division:

Deljenje.h:

#pragma once
#include "Operacija.h"
#include "Mnozenje.h"


class Deljenje : public Operacija
{
public:
    Deljenje(int a);
    int DoOperation(int op1, int op2);
};

Deljenje.cpp:

#include "Deljenje.h"
#include "Mnozenje.h"
class Mnozenje;  

Deljenje::Deljenje(int a)
{
    Mnozenje* b = new Mnozenje(a);
    naziv = "Deljenje";
    suprotna = b;
    if (a == 0)
    {
        cout << "operand ne sme biti 0, stoga je stavljen na default tj.     postavljen na vrednost 1!" << endl;
        drugiO = 1;
    }
    else
        drugiO = a;
}

int Deljenje::DoOperation(int op1, int op2)
{
    int a = 0;
    a = op1 / op2;
    return a;
}

I do understand why this does not work, and I did try with just declaring pointers instead of initialising suprotna member for Mnozenje and Deljenje. But I have another class being a container of an array of type Operacija and I can't have uninitialised pointers, because I need to call some other function using suprotna.

Christophe
  • 68,716
  • 7
  • 72
  • 138
  • _"Im in such a hurry, my exam is in 10-12 hours."_ Well, StackOverflow isn't meant as the last resort to get you through your exam! Also have a look at this possible duplicate for your question please: http://stackoverflow.com/questions/625799/resolve-circular-dependencies-in-c – πάντα ῥεῖ Aug 20 '15 at 21:01
  • I just pointed that out to justify any unclearness and so on, this is a question just like any other. Thanks for tip! :) – MilanMarkovicDrmr Aug 20 '15 at 21:08
  • 2
    It's a low quality question judged by SO's policies though. No matter how urgent it seems from your point of view to get it answered. StackOverflow is for setting up a long term valid Q&A format, and not to serve as your personal help-desk! – πάντα ῥεῖ Aug 20 '15 at 21:10
  • 2
    Can you explain what is not working? – Umashankar Das Aug 20 '15 at 21:22
  • @UmashankarDas firstly thank you for your time. What does not work is that i have another function that is a container for objects 'Operacija' which is a stack, so i need to create that stack and then call 'DoOperation' func for elements of that stack, and then i have func undo in that same class and i need to call it, heres its implementation 'void Kalkulator::Undo() { this->acc = this->niz[this->brel-1]->retOp()->DoOperation(this->acc, this->niz[this->brel - 1]->retD()); this->brel--; }' and what is not working here is that retOp() returns loose pointer,bcs i did not initialize it... – MilanMarkovicDrmr Aug 20 '15 at 21:38

2 Answers2

2

There are three problems raised by your question:

  • The first is about compiling errors, due to the unnecessary circular refecences. Implementing the answer of NO_NAME completely solve this topic as demonstrated here.

  • The second is a segmentation fault that will occur everytime you create a Mnozenje or a Deljene: for example, if you create a Mnozenje from an int, then you'll create right at the start of its construction a new object Deljenje which will create right at the start of its construction a new Mnozenje, which will create ... until a stackoverflow occurs.

  • The third problem is your intended use of inheritance. Having a container of Operacija instead of Operacija* will result in slicing and it's doomed to fail.

Suggestions:

I understand that Mnozenje is multiplication and Deljene a division, and that you try to store in suprotnathe reverse operation.

I propose you therefore to foresee a constructor with an optional argument, so that when you create a reverse operation you can indicate the original operation as reverse.

The idea would be something like this:

class Mnozenje : public Operacija
{
public:
    Mnozenje(int b, Operacija* rev=nullptr);
    int DoOperation(int op1, int op2);
};

Mnozenje::Mnozenje(int b, Operacija* rev)
{
    naziv = "Mnozenje";
    suprotna = rev==nullptr ? new Deljenje(b, this) : rev;   
    ...
}

// and the same kind of change for Deljenje

In this case, instead of recursing for ever, you create the circular reference between the related objects.

ATTENTION: This work-around will make the destruction of the objects slightly more complex. If you have enough time for your exam, consider changing the design:

  • keep two pointers for the reverse operation: one for newly allocated objects, one for pointing back to original operation. In this case you could further improve, using shared_ptr and weak_ptr to help you to manage memory;
  • or create a reverse operation only when it is needed (i.e. not at construction, but as result of a member function);
  • or decouple in your data structure the operations from the operand;
  • or avoid completely creating reverse operation by adding an UndoOperation() member function.
Christophe
  • 68,716
  • 7
  • 72
  • 138
  • 1
    I was kidding about destructor, I now got the whole thing running smoothly, thanks to you. a simple array deletion worked. 'delete [] "var_name"' – MilanMarkovicDrmr Aug 20 '15 at 22:44
1

You don't need #include "Mnozenje.h" in Deljenje.h and #include "Deljenje.h" in Mnozenje.h. The fact that you are using a class in a .cpp file doesn't mean that you have to have definition of this class in a .hpp file.

You also don't need to write declarations of a classes like that: class Deljenje;, because includes (e.g. #include "Deljenje.h") provides you definitions of these classes.

It seems that not quite aware of the fact how includes works. It's pretty simple. Include just copy whole specified file to the place where is an #include directive. You can read more about that here: How does the compilation/linking process work?

Community
  • 1
  • 1
Piotr Siupa
  • 3,929
  • 2
  • 29
  • 65