PDA

View Full Version : Random code generator - is it written alright?




JonnyThunder
Sep 10, 2008, 05:59 AM
Hello,

I'm just starting out with Obj-C / Cocoa (from having done no previous C or C++ programming). I wrote this to generate a random number in a window and wondered if it was a tidy enough way to achieve the goal....


{See next post with code}


This objective-C thing is hard work man! Seriously.

This code does work, but wondered if it is better written another way? Also, if you click the button twice quickly - the random number isn't random (I think because the seed is from 'time'). Is there a better way to generate a better seed (perhaps using Milliseconds?)


EDIT: I changed the seed line to the following, which gives better results when updating fast...


srandom(time(NULL)+(random()%100)+1);



JonnyThunder
Sep 10, 2008, 07:41 AM
OK, I slightly modified it to have an additional output box and checkbox. Can anyone tell me if this is the right approach? I don't want to learn bad habits at this early stage!

[MyAppClass.h]

#import <Cocoa/Cocoa.h>

@interface MyAppClass : NSObject {

IBOutlet NSTextField *myTextField;
IBOutlet NSTextField *myTextField_Reverse;
IBOutlet NSButton *showInReverse;
int outputLength;

}

-(IBAction)randomword:(id)sender;

@end


[MyAppClass.m]

#import "MyAppClass.h"

@implementation MyAppClass

-(void)init
{
// Length of string output
outputLength = 16;
}

-(void)awakeFromNib
{
[myTextField setStringValue:@"Click button to generate"];
[myTextField_Reverse setStringValue:@""];
}

-(void)randomword:(id)sender
{

// Define an array for string
NSMutableString *outputString = [[NSMutableString alloc] initWithCapacity:outputLength];

// Define an array for reverse string
NSMutableString *outputString_Reverse = [[NSMutableString alloc] initWithCapacity:outputLength];

// Seed generator
srandom(time(NULL)+(random()%time(NULL))+1);

// Generate string
for (int i=0; i < outputLength; i++)
{

// Append to string
[outputString appendFormat:@"%c",(random()%26)+65];

}

// Check if we want to display reversed string
if ([showInReverse state]) {

for (int i=0; i < outputLength; i++)
{

// Character by character addition of string (in reverse)
[outputString_Reverse appendFormat:@"%C", [outputString characterAtIndex:((outputLength-1)-i)]];

}

} else {

// Set string blank for display
[outputString_Reverse setString:@""];

}

// Populate output boxes here
[myTextField setStringValue:outputString];
[myTextField_Reverse setStringValue:outputString_Reverse];

}

@end

robbieduncan
Sep 10, 2008, 07:49 AM
Your original seed was correct. But you do it in the wrong place. You seed once: on application startup, not everytime you ask for a random number.

JonnyThunder
Sep 10, 2008, 08:02 AM
I did originally have it in the init method, but it causes a compile time error. Where should I be placing it?

I notice that placing it in awakeFromNib works fine, but thought init method would be a better place...

robbieduncan
Sep 10, 2008, 08:03 AM
I did originally have it in the init method, but it causes a compile time error. Where should I be placing it?

What error? There is no difference to the exact same code (to the compiler) being in init or where you have it. You must have made a mistake.

JonnyThunder
Sep 10, 2008, 08:09 AM
When I have my code like this....


-(void)init
{
// Length of string output
outputLength = 16;

// Seed generator
srandom(time(NULL));
}


I get the following in the debugger console...


Loading program into debugger…
GNU gdb 6.3.50-20050815 (Apple version gdb-960) (Sun May 18 18:38:33 UTC 2008)
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for details.
This GDB was configured as "i386-apple-darwin".Program loaded.
sharedlibrary apply-load-rules all
Attaching to program: `/Users/JT/Documents/Cocoa Programming Work/JTTest/build/Debug/JTTest.app/Contents/MacOS/JTTest', process 2798.

JonnyThunder
Sep 10, 2008, 08:11 AM
Just noticed that if I call the superclass init AFTER my srandom statement, it works fine.

But my question now is, if I didn't need to call super init before - why is it required like this now....

[this code works]

-(void)init
{
// Length of string output
outputLength = 16;

// Seed generator
srandom(time(NULL));

[super init];
}

robbieduncan
Sep 10, 2008, 08:18 AM
You should always call super init first. Without that your object will not be correctly setup.

JonnyThunder
Sep 10, 2008, 08:40 AM
Yes, that's what I thought (and read). But when I put [super init] to the top of the init method, it causes the same issue as I posted before.

I tried to attach the project (2.2mb) but uploads only allow just over 1mb.


-(void)init
{
// Call superclass init method
[super init];

// Length of string output
outputLength = 16;

// Seed generator
srandom(time(NULL));
}


The code above, causes the same compile issue as I posted earlier. However, the code that follows compiles and works no probem. I don't understand why!


-(void)init
{
// Length of string output
outputLength = 16;

// Seed generator
srandom(time(NULL));

// Call superclass init method
[super init];
}

robbieduncan
Sep 10, 2008, 08:46 AM
1) That's not a compiler error: that's a runtime error. It's important to know the difference.

2) Your init function is declared as void. This is wrong.


// Basic init
-(id) init
{
if ([super init])
{
// Do your initialisation here
return self;
}
return nil;
}


This is such a basic, fundamental error I suggest you need to go back to the beginning and read the Objective-C language guide (http://developer.apple.com/documentation/Cocoa/Conceptual/ObjectiveC/Introduction/chapter_1_section_1.html).

JonnyThunder
Sep 10, 2008, 09:00 AM
This is such a basic, fundamental error I suggest you need to go back to the beginning

This was a basic, fundamental mistake - not a lack of understanding. I just didn't see where my mistake was. Considering I never wrote a line of C, Obj-C or anything else object related prior to this script - I think I did alright... no need to be horrible about it!

robbieduncan
Sep 10, 2008, 09:03 AM
It shows a fundamental lack of conceptual understanding about what the init method does and how it works. If you haven't read the document I linked you should: it teaches you about the language you are using and explains it's features and usage. If you don't understand what you are doing you'll never do it right...

JonnyThunder
Sep 10, 2008, 09:19 AM
No... it shows a silly mistake, NOT a misunderstanding of return types, init methods, inheritance or anything else that apparently I'm deemed to be clueless about.

Can we kiss and make up now??? :D

gnasher729
Sep 10, 2008, 11:06 AM
// Basic init
-(id) init
{
if ([super init])
{
// Do your initialisation here
return self;
}
return nil;
}


The pattern that you should always use (unless there is a very good reason to do otherwise):
if (self = [super init]) { ... }
[super init] does not necessarily return self; it could return a different value, and you should use that value instead of self.

robbieduncan
Sep 10, 2008, 11:09 AM
The pattern that you should always use (unless there is a very good reason to do otherwise):
if (self = [super init]) { ... }
[super init] does not necessarily return self; it could return a different value, and you should use that value instead of self.

Very true. I was typing from memory on a PC :o