0

Since desperate times require cranky measures I implemented the following in C++.

-- There is a class called foo. I need an instance (object or reference and not a pointer) of foo in a class called bar.
-- I do not want to declare foo as an object in bar.h since I don't want to include foo.h in bar.h. The reason being bar.h is included in about a hundred .cpp files in the project and including foo.h in bar.h drastically increases the compilation time and brings in spaghetti.
-- I do not want to class forward foo and declare it as a pointer since we are trying to avoid pointers like the plague.
-- The constructor of bar does not pass an instance of foo by argument.

So I did this:

This is foo.h (foo.cpp is a lot of math but is not important in this argument):

#ifndef FOO_H
#define FOO_H

class bell;

class foo {
public:
    foo(const bell& bell_i);
    virtual ~foo();
    double foo_member(); /*does math in foo.cpp*/
}; 
#endif  /* FOO_H */

This is bar.h:

#ifndef BAR_H
#define BAR_H

class bell;
class foo;

class bar {
public:
    bar(const bell& bell_i);
    virtual ~bar();
    double bar_member(); /* does math in bar.cpp and uses foo */
private:
    foo& myfoo;
}; 
#endif  /* BAR_H */

This is bar.cpp:

#include "bell.h"
#include "foo.h"

bar::bar(const bell& bell_i) : myfoo(*(new foo(bell_i))) /* NOTE: this construction is my question. */
{}

bar::~bar()
{}

double bar::bar_member()
{
    return sqrt(myfoo.foo_member());
}

So, in essence, I am creating a pointer to foo, taking the object from it and initializing myfoo. This code and all associated code compiles perfectly and runs without any errors. I have tested it with multiple compilers.

My questions are:

-- Is there anything wrong code-wise (syntax/lifetimes/dangling pointers etc.) with initializing myfoo as such?
-- Is there anything wrong codeologically with doing this?
-- Am I missing something?
-- Is there an alternative without using pointers?

If you want to see the real code it is here: the .cpp file, the .h file.

Thanks in advance. I am a physicist. So, please excuse my code lingo.

Update: The leaks did not seem to be a problem since only a few instances of the class were constructed at initiation and not destroyed during runtime and only destroyed at exit. But this should not be done for classes that get created and destroyed during runtime.

talismanbrandi
  • 116
  • 4
  • 6
    Your code has a memory leak. – juanchopanza Jun 20 '17 at 12:55
  • 1
    Not really answering your question, but why do you avoid pointers like plague? They're not bad at all. Maybe you're thinking about raw pointers that actually *own* an object. That you should avoid like plague, but for the rest, pointers are perfectly fine. – Rakete1111 Jun 20 '17 at 12:57
  • Every construction of bar leakes memory in your implementation – xEric_xD Jun 20 '17 at 12:58
  • 4
    @talismanbrandi It clearly does. You looked wrong. – juanchopanza Jun 20 '17 at 12:59
  • @talismanbrandi Yet you decide to use `new` without knowing how to. You're replacing a non-problem with an actual one. – juanchopanza Jun 20 '17 at 13:03
  • 2
    @talismanbrandi You are deluding yourself. Your code has a memory leak. End of. – juanchopanza Jun 20 '17 at 13:04
  • @talismanbrandi just curious, if I understood right, you are a physicist in need of computing some math. Did you consider using Matlab of it's free alternative Octave instead of C++? These could be better suited for your needs. – Michael232 Jun 20 '17 at 13:05
  • Use a `unique_ptr` and stop this madness. You're adamant about not having a memory leak, but you clearly do. You use `new` without `delete`. If you were sure you would not ask "Is there anything wrong code-wise (syntax/lifetimes/dangling pointers etc.) with initializing myfoo as such?", and you would accept that the answer is yes. – Retired Ninja Jun 20 '17 at 13:09
  • If you're so positive that you do not have a leak why would you ask about it? If you're so positive your way is acceptable then why did you ask this question at all since you're not willing to accept any answer other than what you've already done? I have 30 years of experience, 29 of which are not necessary at all to know if you allocate memory and do not free it you have a leak, end of story. – Retired Ninja Jun 20 '17 at 13:15
  • Here's a simple example of your memory leak. You can accept it or not, but it is there. http://ideone.com/23b5ET – Retired Ninja Jun 20 '17 at 13:23

2 Answers2

4

Your measures defy purposes here. Instead of pointer you now have a memory leak as you allocate memory for foo instance anyway, but you never free it. If you need to use pointer/reference, but don't want to keep track of the object lifetime manually, you better use RAII approach, for example, std::unique_ptr. It will hold your object pointer and carefully deallocate it when the time comes.

Michael232
  • 223
  • 1
  • 6
  • 1
    @talismanbrandi for example, check this code: http://cpp.sh/7nyvn If you run it you can see that foo instance created on stack (number 1) is constructed and then destroyed. Instance 2, which is created same as in your code, is never destroyed. – Michael232 Jun 20 '17 at 13:14
  • @talismanbrandi It's worth mentioning that if you want to deal with the management of memory manually (a bad idea, in general), deleting the object in the destructor isn't enough to solve the problem. You also have to deal with the copy constructor and copy assignment, and if you use C++11 or above also the move constructor and move assignment. This is generally referred to as the [rule of three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three), or [rule of five](https://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11) in C++11. – Fabio says Reinstate Monica Jun 20 '17 at 14:16
  • thanks @FabioTurati. We are trying to minimize manual memory management. – talismanbrandi Jun 20 '17 at 14:27
0

Use a pointer as the member, but have an accessor Foo &GetFoo() {return *pmyFoo;} that returns a reference. Make the member private and use the accessor everywhere, even within the class's code.

Except for that one method (oh, and the dtor to free it up), as far as all of your code is concerned, there is no pointer, only a reference.

franji1
  • 3,088
  • 2
  • 23
  • 43