0

!! Note: I edited the question many times after the answers. But the question below is the first question and the first answer is a helpful answer. Please do not confuse by some comments. They are written after I changed my question many times.


I have a Shop template class and have class Cookie. Shop is a list that keeps cookies named cookieShop. Shop cotr can take one cookie as its parameter and more can be added by Add method of the Shop template class.

I'm creating two cookies. One is added by shop cotr, and second by add method. Even if I dont write codes in add method, second cookie is added to shop list. I tried to understand why it does that but could not.

Here is my code:

//Shop.h

#ifndef SHOP_T
#define SHOP_T
#include <string>
using namespace std;

template<class type>
class Shop;

template<typename type>
ostream& operator<<(ostream& out, const Shop<type>& S){
    for(int i = 0; i < S.size; i++)
        out << i + 1 << ".\t" << S.list[i] << endl;
}

template<class type>
class Shop{
    type *list;
    string name;
    int size;
public:
    Shop() { list = 0; size = 0; }
    Shop(type t);
    ~Shop(){ delete[] list; }
    void Add(type A);
    friend ostream& operator<< <>(ostream& out, const Shop<type>& S);
};

template<class type>
Shop<type>::Shop(type t) : size(0){
    list = new type;
    list = &t;
    size++;
}

template<class type>
void Shop<type>::Add(type A){
//  type *temp = new type[size+1];
//  for(int i = 0; i < size; i++)
//      temp[i] = list[i];
//  delete[] list;
//  temp[size] = A;
//  list = temp;
    size++;
}

#endif

//Cookie.h

#ifndef COOKIE
#define COOKIE
#include <string>
using namespace std;

class Cookie{
    string name;
    int piece;
    float price;
public:
    Cookie();
    Cookie(string n, int pi, float pr);
    friend ostream& operator<<(ostream& out, const Cookie& C);
};

#endif

//Cookie.cpp

#include "Cookie.h"
#include <iostream>
#include <string>
using namespace std;

Cookie::Cookie() {}
Cookie::Cookie(string n, int pi, float pr){
      name = n;
      piece = pi;
      price = pr;
}

ostream& operator<<(ostream& o, const Cookie& C){
    o << C.name << "\t" << C.piece << "\t" << C.price;
}

//main.cpp

#include <iostream>
#include <string>
#include "Shop.h"
#include "Cookie.h"

using namespace std;
int main(){
    Cookie cookie1("Chocolate Cookie", 50, 180);
    Cookie cookie2("Cake Mix Cookie", 60, 200);

    Shop<Cookie> cookieShop(cookie1);
    cookieShop.Add(cookie2);

    cout << cookieShop <<endl;


    return 0;
}

As you can see the codes in Add method are commented. How it adds the second cookie?

When compile (gcc main.cpp Cookie.cpp) and run it (./a.out) it gives the lines below:

  1. Chocolate Cookie 50 180
  2. Cake Mix Cookie 60 200 Segmentation fault (core dumped) And why I get Segmentation fault error?

Note: Im new at stackoverflow. If I have wrong behaviors. Please tell :)

Murat
  • 1
  • 3
  • 1
    You forgot to comment out `size++;` in `Add`. – NathanOliver May 16 '17 at 14:25
  • @NathanOliver. Because I want to print list[1]. I think problem is not that. Is it ? – Murat May 16 '17 at 20:42
  • Whoever taught you that raw arrays are a useful container is badly wrong. You are *much* better off using `std::vector` here – Caleth May 16 '17 at 23:55
  • It was a homework. I have to do with this way. Deadline has already ended. Unfortunatelly, I sent the homework as above. But now want to learn. – Murat May 18 '17 at 15:08
  • I applaud your desire to learn, but 1) please do not edit your question in a way that changes them entirely, and 2) when you write code, it is best to start with something small and simple that works perfectly, and when you post code here, it is best to reduce it to the smallest, simplest code that produces the error. – Beta May 18 '17 at 23:36
  • @Beta: Thanks for your suggestions. I have not spent much time in forums and do not know how to do it. I will consider them. But how can find a solution for your first suggestion. According to your answer below, I changed the part of code you specified. How should I do it? – Murat May 19 '17 at 11:44
  • @Beta: I made the question smaller now. Is it better now? – Murat May 19 '17 at 12:12
  • @Murat you can discuss with the author of an answer via comments. Once you are satisfied with the answer, mark it as the correct one. If you discover additional problems, post a new question. – Quentin May 19 '17 at 12:12
  • okey, sorry about it. – Murat May 19 '17 at 12:26

1 Answers1

2

Here:

template<class type>
Shop<type>::Shop(type t) : size(0){
  list = new type;
  list = &t;
  size++;
}

The variable t is local to this function. You make list a pointer to that variable. When the function terminates, the variable passes out of scope, the Shop is left holding a dangling pointer, and you get undefined behavior.

Beta
  • 96,650
  • 16
  • 149
  • 150
  • Oh. I did really bad code here. I will try to correct it. Thanks. – Murat May 16 '17 at 20:47
  • There is also UB from the mismatch of `list = new type`, `list = &t` with `delete [] list` – Caleth May 16 '17 at 23:49
  • @Caleth Why does it still give segmentation fault error after I change the code you mentioned? Am I wrong with the changed code too? – Murat May 18 '17 at 23:06
  • @Murat: Now you invoke your `Add` function, but it is still mostly commented out; it doesn't add a Cookie, it merely increments `size`. Then when you try to print the second Cookie, you read past the end of the array (which is not really an array, a `Cookie` is not a `Cookie[1]`), and get undefined behavior again. – Beta May 18 '17 at 23:39
  • @Beta: I've tried the code by commented the part: `cookieShop.Add(cookie2);` in main.cpp. Sorry, I forgot to change it here. But I get the same result as above. – Murat May 19 '17 at 11:57
  • @Murat: I think it is more important that you understand the past errors, than get the code working. This forum is not designed for iterative troubleshooting. (Don't feel bad about being a beginner, the skills come with practice.) – Beta May 19 '17 at 13:36