invoking Base::virtualMethod() instead of Derived

In my class Js::Object I've an operator() overload which returns Js::JType*
all Other Classes in Js Namespace e.g. Js::Number, Js::String etc.. even Js::Object inherits from Js::JType

Code:

Js::JType* &operator()(const string& key);

source

Code:

/**
typedef std::map<string, Js::JType*> ElemMap;
*/
Js::JType* &Js::Object::operator()(const string& key){
	ElemMap::iterator it=list.find(key);
	if(it != list.end()){
		return (Js::JType* &)(it->second);
	}else{
		Js::JType* x = new Js::JType;
		list.insert(make_pair(key, x));
		return x;
	}
}

and this is how I am using it.

Code:

Js::Object o;
o("math") = new Js::String("Hello World");

Now If everything is ok.

Code:

o("math")->commit()

should return "Hello World" (with quotes as std::string)

then it will invoke Js::String::commit()
But Its invoking Js::JType::commit()

and thus its returning null (as std::string)

e.g. in other words its invoking the commit() of grand parent Class not of the intended class.

and yes commit is virtual.

Hope I am able to clarify the problem.

You are returning a reference to a local pointer in the else case (and the compiler might be able to warn about that with warnings on). Instead you should probably return a reference to the pointer that is held in the map, because that is what you want to modify.

In any case you are leaking memory: who's cleaning up the object allocated by operator() when you immediately assign it another pointer allocated with new.

Now I am using.

Code:

Js::JType* x = new Js::JType;
list.insert(make_pair(key, x));
return list[key];

It fixes the problem and I think its might raise a null pointer issue which is not going to harm in my application.But I don't think it dies memory leak.

Originally Posted by noobcpp

Now I am using.

Code:

Js::JType* x = new Js::JType;
list.insert(make_pair(key, x));
return list[key];

It fixes the problem and I think its might raise a null pointer issue which is not going to harm in my application.But I don't think it dies memory leak.

Insert returns an iterator, so better would be:

Code:

Js::JType* x = new Js::JType;
return list.insert(make_pair(key, x)).second;

Or at least, it wouldn't need the extra lookup . About not dying by memory leaks... well, memory leaks are still really bad. And yes, it *will* die from it if the program runs for too long. It's really advisable to fix it. A simple hack-fix would be to replace the pointers by reference counted pointers, which clean up their memory automatically if they are no longer required.

Not a hack fix. A valid solution, actually. It recommended to use smart pointers as much as possible.

But using smart pointers increases syntactic complexity.
even if I design a macro for general pointer to auto_ptr you still have to type the macro name.
its not an automatic job.

Syntactic complexity is hardly a convincing argument not to use a smart pointer.

One solution probabbly can do coertion
If I've

Code:

operator std::auto_ptr<Class>(){
  return std::auto_ptr<Class>(this);
}

But is this solution recomendable ??

and if all my functions take auto_ptr<> as argument it will automatically be converted.

I don't think auto_ptr's are versatile enough to solve all memory management issues.

In any case, why can't you have two separate methods for getting and setting values?

Oh my god, you're overcomplicating this method. The correct implementation for operator() is :

Code:

Js::JType*& Js::Object::operator()(const string& key){
  return list[key];
}

This returns a null pointer if the object isn't in there yet, but if you ask me, that's preferable to a newly created JType.

This leaves open the question whether this the right approach at all.

CornedBee: Yes, that is what rip-off suggested to him on Gamedev.

How would I know?

That's just why we discourage cross-posting here.