0

I'm trying to implement a SmartPtr class in my code that used to use normal pointers. I've looked at other sof questions and their solutions seem to be what I'm doing, so I'm not sure what's wrong. I had to define my global in graph.h because a function parameter, shown, uses it.

./graph.h:14:1: error: unknown type name 'null_adj'
null_adj.id = -1;
^
./graph.h:14:9: error: cannot use dot operator on a type
null_adj.id = -1;

        ^
2 errors generated.

I define it in graph.h:

#include "node.h"
#include "SmartPtr.cpp"
using namespace std;

Adjacency null_adj;
null_adj.id = -1;
SmartPtr<Adjacency> null(&null_adj);

class Graph { ...
    void insert_and_delete(stuff, SmartPtr<Adjacency> second_insert = null); ...

This is node.h:

    #include "SmartPtr.cpp"
#include <vector>

using namespace std;

struct Adjacency{
public:
    int id;
    char letter;
    int type;
};

class Node { ...

SmartPtr.cpp:

    #ifndef SMARTPTR_CPP
#define SMARTPTR_CPP

#include<iostream>
using namespace std;

// A generic smart pointer class
template <class T>
class SmartPtr
{
   T *ptr;  // Actual pointer
public:
   // Constructor
   explicit SmartPtr(T *p = NULL) { ptr = p; }

   // Destructor
   ~SmartPtr() { delete(ptr); }

   // Overloading dereferncing operator
   T & operator * () {  return *ptr; }

   // Overloding arrow operator so that members of T can be accessed
   // like a pointer (useful if T represents a class or struct or
   // union type)
   T * operator -> () { return ptr; }
};

#endif

What is wrong ??? Edit: look at the follow up in the little box below.

Elle Szabo
  • 21
  • 6
  • You'd really need to include a [mcve] complete with the actual error, but you're attempting to use the arrow operator on your `Adjacency` object which is not a pointer: `null_adj->id`. Should that be `nujll->id` instead? – Tas Dec 28 '19 at 06:11
  • You're totally right, it should be a dot operator. But now I'm getting the same errors: ./graph.h:14:1: error: unknown type name 'null_adj' null_adj.id = -1; ^ ./graph.h:14:9: error: cannot use dot operator on a type null_adj.id = -1; @Tas – Elle Szabo Dec 28 '19 at 06:13
  • 2
    You can't put code like `null_adj.id = -1;` in the global namespace (that is, outside a function or `main`). If you want to give the `id` member a *default* value, then write it into the declaration of `Adjacency` as … 'int id = -1;`. – Adrian Mole Dec 28 '19 at 06:16
  • Probably unrelated, but `#include "SmartPtr.cpp"` is a bad plan. Don't include the cpp files. Compile and link the suckers. Include the headers. Looking deeper, it looks like you've covered multiple inclusion, but but a lot of build tools will still choke over this. I recommend changing the name to SmartPtr.hpp or something similar. – user4581301 Dec 28 '19 at 06:19
  • Do you want to use some kind of smart pointers in a real project or are you implementing your `SmartPtr` class just to figure out how a smart pointer could be implemented. Because if you want to use them in a real project then you really should use `std::unique_ptr` and `std::shared_ptr` instead. – t.niese Dec 28 '19 at 06:30
  • @AdrianMole ok thanks. is that syntactical or stylistic? – Elle Szabo Dec 28 '19 at 06:32
  • @user4581301 i knew that but wanted a quick implementation... i'll try to link them. thanks! – Elle Szabo Dec 28 '19 at 06:32
  • 1
    Don‘t use `using namespace` in a file that you `#include` somewhere else. You smart pointer has to be change to follow the rule of three (or even better the rule of five), in the current form the implementation can result in double delete and use after delete errors. – t.niese Dec 28 '19 at 06:33
  • @t.niese I want to use them in a real project... they are much needed. code won't compile because of multiple deletes, pain in the ***. I did try to use unique_prt but all of the constructors are private! It didn't seem like an easy problem to fix. – Elle Szabo Dec 28 '19 at 06:34
  • @t.niese explain how it can result in double delete? – Elle Szabo Dec 28 '19 at 06:35
  • 1
    Explanation of t.niese's point: [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) Reason I recommend renaming the SmartPtr.cpp rather than trying to link it file: [Why can templates only be implemented in the header file?](https://stackoverflow.com/questions/495021/why-can-templates-only-be-implemented-in-the-header-file) – user4581301 Dec 28 '19 at 06:48
  • `SmartPtr` still has a default copy constructor and assignment operator. So if you have something like `SmartPtr a1(new Adjacency())` and then some `SmartPtr a2(a1)` (or when you e..g pass it by value to another function), then both `a1` and `a2`, hold the same pointer, so both will call `delete` on the same pointer resulting on double delete. – t.niese Dec 28 '19 at 07:24
  • And never do `Adjacency null_adj; SmartPtr null(&null_adj);` (that is also true for `std::unique_ptr` or `std::shared_ptr`, because the lifetime of `Adjacency null_adj;` is controlled by the scope, so `null_adj` is automatically deleted when leaving the scope, and then another time by your `SmartPtr`. (`delete` has to be only called on memory allocated with `new`). – t.niese Dec 28 '19 at 07:32
  • `I did try to use unique_prt but all of the constructors are private! It didn't seem like an easy problem to fix.` then you should ask a question here on SO about how to solve that problem. Ignoring the use of `friend` which should not be part of a smart ptr, then I can't imagine any situation where an own valid implementation of a smart pointer set, would solve a problem about `private` or `protected` that you have with `std::unique_ptr` and `std::shared_ptr`. – t.niese Dec 28 '19 at 07:33
  • 1
    I highly recommend, to solve the problems you have with using `std::unique_ptr` and `std::shared_ptr` in your use-case, instead of implementing your own smart pointer. Even the std library didn't get their smart pointer implementation _"right"_ in the first try (their first attempt was the now deprecated `std::auto_ptr`), so you shouldn't expect that it is an easy task to get them right. – t.niese Dec 28 '19 at 07:37
  • @t.niese thank you for the explanations. I'll look into unique_ptr once more – Elle Szabo Dec 28 '19 at 23:31

1 Answers1

0

You can have only declarations at global scope:

Adjacency null_adj;
null_adj.id = -1;

The first of these two lines is a declaration, the second is not. The way to avoid the need to have expressions at global scope is to encapsulate the operation into a function, e.g., a constructor. That would also allow the use of the object initialization without the need of a separate object. With you current class you can use structured initialization:

SmartPtr<Adjacency> sp(new Adjacency{-1});

The next step is then to avoid global variables entirely: they generally cause problems: I’d recommend against global variables and if they are consider essentially I’d recommend to make the constexpre or, at least, const. Note that const objects at global scope already suffer from an indeterministic initialization order.

Dietmar Kühl
  • 150,225
  • 13
  • 225
  • 380
  • Thank you! Now I'm getting "ld: symbol(s) not found for architecture x86_64." I hadn't gotten that error before. Tried writing a new, simple makefile but got the same error. What's going on? – Elle Szabo Dec 29 '19 at 01:19