Is this dangerous or bad practice?

Code:

---

Level* LoadLevel(const string& fileName)
{
  Level& level = *(new Level(GetFileModTime(fileName)));
  ....
  return &level;
}

---

instead of

Code:

---

Level* LoadLevel(const string& fileName)
{
  Level* level = new Level(GetFileModTime(fileName));
  ....
  return level;
}

---

I just need to use operators on the level object, which is more annoying when its a pointer.

I don't see any point in doing the first variant - as to if it's dangerous or not - can't say for sure - I think not.

--
Mats

That looks to me like simply an unusual combination of a number of not-so-unusual things. I would say it is absolutely not dangerous, but whether it is bad practice it probably a matter of opinion.
I think it's probably okay.

Here's food for thought:

Code:

---

template <typename T> T& new_ref()
{
    return *(new T());
}

template <typename T, typename P1> T& new_ref(P1 p1)
{
    return *(new T(p1));
}

template <typename T, typename P1, typename P2> T& new_ref(P1 p1, P2 p2)
{
    return *(new T(p1, p2));
}

... etc

---

Of course this makes deletion a little unintuitive and easily forgotten, so I wouldn't suggest actually using code like the above.

You might also make a separate reference:

Code:

---

Level* LoadLevel(const string& fileName)
{
  Level* level = new Level(GetFileModTime(fileName));
  Level& level_ref = *level;
  ....
  return level;
}

---

You return a Level* in both cases. The first one won't compile, will it?

^ It compiles fine and it should. I just wanted to know if there was some hidden danger doing it like this. The function LoadLevel is alot bigger than what i posted, and it uses operators like [] and <<. I didnt want to have to convert it to a reference each time i need to call an operator (like (*level)[4] = blah)

Quote:

---

Originally Posted by 39ster

^ It compiles fine and it should. I just wanted to know if there was some hidden danger doing it like this. The function LoadLevel is alot bigger than what i posted, and it uses operators like [] and <<. I didnt want to have to convert it to a reference each time i need to call an operator (like (*level)[4] = blah)

---

So, use anon's method. Also, you may find that you want to actually make it into two functions, where you allocate the pointer, and then pass the pointer by reference to a "fill in" function.

--
Mats

It might be best to return a pointer to dynamically allocated memory. It makes it a little more intuitive that the caller needs to deallocate the memory. If they receive a reference, I'm not sure if it's so obvious.

But, if you return a pointer to an object which has been created by 'new' in a function, that pointer won't point to the right object? I remember this problem I had before, I had to solve it with " ::new " (using anon's method):

Code:

---

Level* LoadLevel(const string& fileName)
{
  Level* level = ::new Level(GetFileModTime(fileName));
  Level& level_ref = *level;
  ....
  return level;
}

---

ref: http://blogs.msdn.com/jaredpar/archi...ement-new.aspx

I think you're completely wrong on this, unless I'm misunderstanding you. That article has to do with placement new.

hmm.. ok, sorry for that. The article is the wrong one, can't find the right one anymore!

Quote:

---

Originally Posted by 39ster

^ It compiles fine and it should.

---

Sorry - I completely missed that you were returning the address of level in the first one.

Both variants are exception-unsafe and do not indicate clearly that the return value must be freed by the user. In addition, the second is unusual and thus a detriment to readability.

You should:
1) Capture the pointer in a smart pointer immediately.
2) If you want reference access, create a separate reference.
3) Return the smart pointer.
E.g.

Code:

---

std::auto_ptr<Level> LoadLevel(const string& fileName)
{
  std::auto_ptr<Level> plevel(new Level(GetFileModTime(fileName)));
  Level &level = *plevel;
  ....
  return plevel;
}

---

IMalc, thanks for the site in your sig!