PDA

View Full Version : Objective C++ class question




Soulstorm
Aug 27, 2007, 02:24 AM
I've been messing with ObjC with C++ for quite some time now, and I am building a framework to unify those two languages.

I have a C++ class that contains an ObjC class like this:

class SFMutableString{
public:
NSMutableString *_str;


SFMutableString(){_str = [NSMutableString new];}
~SFMutableString(){
[_str release];
NSLog(@"str deallocating....");
}

SFMutableString(const char* aString){
[_str setString:[NSMutableString stringWithCString:aString encoding:NSUTF8StringEncoding]];
}
SFMutableString(const SFMutableString& other){
_str = [NSMutableString stringWithString:other._str];
}
SFMutableString(NSString* aString){
_str = [NSMutableString stringWithString:aString];
}
void getString(const char* aStr){
[_str setString:[NSMutableString stringWithCString:aStr encoding:NSUTF8StringEncoding]];
}

SFMutableString operator=(SFMutableString& other){
[_str setString:other._str];
return *this;
}
SFMutableString operator=(const NSString *otherString){
[_str setString:[NSMutableString stringWithString:otherString]];
return *this;
}

};

And I have this simple main():
int main (int argc, const char * argv[]) {
SFMutableString str = @"hello world!";
NSLog(str._str);
return 0;
}


The code compiles OK, but on runtime, I get this error:
[Session started at 2007-08-27 10:19:08 +0300.]
2007-08-27 10:19:08.245 foundation tool 1[1100] *** _NSAutoreleaseNoPool(): Object 0x605050 of class NSCFString autoreleased with no pool in place - just leaking
2007-08-27 10:19:08.245 foundation tool 1[1100] hello world!
2007-08-27 10:19:08.245 foundation tool 1[1100] str deallocating....

foundation tool 1 has exited with status 0.

I tried using an Autorelease pool, but then I get errors about double deallocation... Any ideas on why this happens and what can I do?

EDIT: I tried using an autorelease pool, but I had to remove the [release str] command from the deallocation method of the class. But somehow, I don't feel that this is the way it is supposed to be done. I mean, if I implement this class as it is (without the release command on deallocation, leaving the job to be done by an autorelease pool) on a Cocoa program, won't I get memory leaks?



gnasher729
Aug 27, 2007, 06:10 AM
1. The SFMutableString(const char* aString) constructor is asking for trouble. I think it is likely to just crash.

2. The assignment operators return the object *this. That is a highly unusual thing to do, because it means a copy constructor will be called for every assignment. Usually you would return just a reference to an object, not the object itself.

Soulstorm
Aug 27, 2007, 10:18 AM
1. The SFMutableString(const char* aString) constructor is asking for trouble. I think it is likely to just crash.

2. The assignment operators return the object *this. That is a highly unusual thing to do, because it means a copy constructor will be called for every assignment. Usually you would return just a reference to an object, not the object itself.

1. Why? It seems to work just fine for me
2. OK, you are right, I corrected that, thanks.

But, unfortunately, that doesn't answer my original question, since the problem remains...

kainjow
Aug 27, 2007, 10:28 AM
Memory rules still apply. You are setting _str to an autoreleased object almost every time. You need to retain it.

And you do need an NSAutoreleasePool if you're using autorelease methods.

whooleytoo
Aug 27, 2007, 11:14 AM
The "const char*" constructor won't work, as you're calling a method of _str (setString: ) before it has been created.

Personally, I'd have just one constructor (probably one taking an NSMutableString* parameter) which actually creates the _str object; and all the other constructors then call this one.

Create _str using alloc & initWithString: (e.g.

_str = [[NSMutableString alloc] initWithString: inString];


and release it in the destructor, and you're done.

Soulstorm
Aug 27, 2007, 01:09 PM
Memory rules still apply. You are setting _str to an autoreleased object almost every time. You need to retain it.

And you do need an NSAutoreleasePool if you're using autorelease methods.

OK, let's consider the following class:
class SFMutableString{
public:
NSMutableString *_str;


SFMutableString(){
strAlloc();
NSLog(@"default constructor");
}
~SFMutableString(){
NSLog(@"trying to deallocate... retain count: %i",[_str retainCount]);
[_str release];
}

void strAlloc(){
if (_str == nil){
_str = [[NSMutableString alloc]init];
NSLog(@"allocation was necessary");
}
else
NSLog(@"no allocation was necessary");
}

SFMutableString(const char* aString){
NSLog(@"constant char constructor");
strAlloc();
[_str setString:[NSMutableString stringWithCString:aString encoding:NSUTF8StringEncoding]];
}
SFMutableString(const SFMutableString& other){
NSLog(@"copy constructor");
strAlloc();
[_str setString:other._str];
}
SFMutableString(NSString* aString){
NSLog(@"NSString constructor");
strAlloc();
[_str setString:aString];
}
void getString(const char* aStr){
[_str setString:[NSMutableString stringWithCString:aStr encoding:NSUTF8StringEncoding]];
}

SFMutableString& operator=(SFMutableString& other){
NSLog(@"SFMutableString operator");
strAlloc();
[_str setString:other._str];
return *this;
}
SFMutableString& operator=(const NSString *otherString){
NSLog(@"NSString operator");
strAlloc();
[_str setString:[NSMutableString stringWithString:otherString]];
return *this;
}
SFMutableString& operator=(const char* aString){
NSLog(@"constant character operator");
strAlloc();
[_str setString:[NSString stringWithCString:aString encoding:NSUTF8StringEncoding]];
return *this;
}

//getters
char* c_str(){
char *result = new char [[_str length]];
strcpy(result,[_str cStringUsingEncoding:NSUTF8StringEncoding]);
return result;
}
};

and the following main:
int main (int argc, const char * argv[]) {
NSAutoreleasePool *pool = [NSAutoreleasePool new];

SFMutableString str = @"hello world!";
NSLog(str._str);

[pool release];
return 0;
}

The compiler gives me the correct results. If I remove the autorelease pools, I will get runtime errors. You said that I need to use an autorelease pool. In terms of good programming practice, do I need to do that in this occasion or must I alter my class for not using that?

I know that Cocoa applications (the main() function I give you is from a foundation tool) use their own autorelease pools. Does this mean that I won't need to define one, and this class will work OK?

kainjow
Aug 27, 2007, 01:18 PM
The compiler gives me the correct results. If I remove the autorelease pools, I will get runtime errors. You said that I need to use an autorelease pool. In terms of good programming practice, do I need to do that in this occasion or must I alter my class for not using that?

I know that Cocoa applications (the main() function I give you is from a foundation tool) use their own autorelease pools. Does this mean that I won't need to define one, and this class will work OK?

Basically, wherever you have Cocoa code, you need an NSAutoreleasePool because you don't always know what the Cocoa code is doing, and it most likely is using autorelease methods within. If you use autorelease methods without an autorelease pool, you will create leaks.

NSAutoreleasePool *pool = [NSAutoreleasePool new];
I would not use new. It is old school. Use alloc/init instead.

iSee
Aug 27, 2007, 01:46 PM
OK, let's consider the following class:

The compiler gives me the correct results. If I remove the autorelease pools, I will get runtime errors. You said that I need to use an autorelease pool. In terms of good programming practice, do I need to do that in this occasion or must I alter my class for not using that?

I know that Cocoa applications (the main() function I give you is from a foundation tool) use their own autorelease pools. Does this mean that I won't need to define one, and this class will work OK?

I'm too much of a noob at Objective-C to comment directly but I think this has a nice explanation of everything:

http://developer.apple.com/documentation/Cocoa/Conceptual/MemoryMgmt/index.html

Read (at least :) ) the first couple sections which I think directly applies in your situation. Also, there is a whole section on Autorelease Pools and a bunch of other stuff.

Also, your new code makes me nervous because _str is not explicitly initialized before calling strAlloc() in the constructors (the first thing strAlloc() does is to examine the value of _str). I don't know Objective-C++ well enough to know if _str is guaranteed to be initialized to nil for you (definitely not in regular C++) so I mention it, just in case.

kainjow
Aug 27, 2007, 01:53 PM
Also, your new code makes me nervous because _str is not explicitly initialized before calling strAlloc() in the constructors (the first thing strAlloc() does is to examine the value of _str). I don't know Objective-C++ well enough to know if _str is guaranteed to be initialized to nil for you (definitely not in regular C++) so I mention it, just in case.

That's a good point. I have recently had problems in Cocoa code because variables weren't initialized to nil. You should always declare your variables to nil.

Soulstorm, if I were you, I'd have only one constructor in your class. Then have normal setters for inputting other types of data. I think it would be a much cleaner design.

MongoTheGeek
Aug 27, 2007, 03:11 PM
I have a question that might seem to be heretical, but why are you doing this. NSStrings and CFStrings are toll free bridged. I can take a CFString and send messages to it like an NSString or call methods of a NSString like it were a CFString.

I've done it some place and I remember having a good reason. Can't remember the reason, can't find the code but...

iSee
Aug 27, 2007, 03:20 PM
I have to say, I prefer a wide variety of constructors in C++ classes that wrap general data types. Though, after years of C++ programming I almost always implement this type of C++ class following this pattern:

I create a private init() method that takes no parameters that performs simple initialization on all member variables. This is always called first in all constructors. Typically, memory is not allocated here--pointers are set to NULL. Constructors that take one parameter just call an assignment operator that takes the same type of parameter. Constructors that take more than one parameter call a "set" function that takes the same parameters. In a class that initializes memory, I also have a "cleanup()" method that frees allocated memory but ensures that freed pointers are set to NULL.

For example (I'm just typing this, it's not a real class):


class MyStringWrapper
{
private:
char* _str;
init() { _str = NULL; }
cleanup() { if (_str != NULL) { delete[] _str; _str = NULL; }

public:
MyStringWrapper() { init(); }
MyStringWrapper(const MyStringWrapper& str) { init(); *this = str; }
MyStringWrapper(const char* pStr) { init(); *this = pStr; }
MyStringWrapper(const char* pStr, int numChars) { init(); setFromChars(pStr, numChars); }
MyStringWrapper(int val) { init(); *this = val; }

~MyStringWrapper() { cleanup(); }

MyStringWrapper& operator=(const MyStringWrapper& str) { cleanup(); ...; return *this; }
MyStringWrapper& operator=(const char* pStr) { cleanup(); ...; return *this; }
MyStringWrapper& setFromChars(const char* pStr, int numChars) { cleanup(); ...; return *this; }
MyStringWrapper& operator=(int val) { cleanup(); ...; return *this; }

...
};



Adhering to this structure keeps the memory allocation/freeing organized into two functions, regardless of how many constructors or other public initializers you might want to create. It also ensures you aren't duplicating logic between constructors and "setters."

whooleytoo
Aug 27, 2007, 03:38 PM
Also, your new code makes me nervous because _str is not explicitly initialized before calling strAlloc() in the constructors (the first thing strAlloc() does is to examine the value of _str). I don't know Objective-C++ well enough to know if _str is guaranteed to be initialized to nil for you (definitely not in regular C++) so I mention it, just in case.

I would guess it's not safe.

In Obj-C code, it's +alloc that initialises instance variables, but since SFMutableString isn't alloced, I'd certainly play it safe.