macOS Randomly generated phone number not being returned properly

[isthisonetaken]

Hey all,

so I'm working on a program that will randomly generate some information to be bulk loaded into a database for school. The first function I'm trying to get working is generating a random phone number.

Here is my thought process for this:
have a for loop run through 12 iterations (0-11)
if the current iteration is 3 or 7 insert a -
else find a random number between 0 and 9, convert the number to a string, append that string onto the phone number string.
return the phone number string

I wrote the code and had it show me the returned phone number in a alert box. My problem is that it is returning phone numbers such as 111-111-1111. I put in a breakpoint to see why it was returning numbers like that and found that it was actually finding random numbers and appending them properly, then returning the wrong string made up of whatever the last digit found was.

Here is the code to generate the phone number:

- (NSString *)randomPhone {
	NSString *phone = [[NSString alloc] initWithString:@""];
	int digit;
	NSString *stringDigit = [[NSString alloc] init];
	
	for (int i = 0; i < 12; i++) {
		if (i == 3 || i == 7) {
			phone = [phone stringByAppendingString:@"-"];
		} else {
			digit = [self randomNumberBetween:0 and:9];
			stringDigit = [NSString stringWithFormat:@"%d", digit];
			phone = [phone stringByAppendingString:stringDigit];
		}
	}
	
	return phone;
}

Does anyone have any idea why it would be returning the wrong string?

Thanks,

Dan

 

[plinden]

How about rethinking how to do this.

Instead generate three random numbers: 100-999, 100-999, 1000-9999

Use

NSString *phone = [NSString stringWithFormat:@"%d-%d-%d", areaCode, phone1, phone2];

 

[lee1210]

I haven't run the code, but straight away it's clear that you're not familiar with retain-release memory management, which is going to be problematic for you whether it's the problem in this chunk of code or not. If you assign a pointer to something returned by a method containing init, new, or copy then you own it. If you assign over that, you're leaking memory. On the other hand, if you get a pointer back from a method that does not contain one of those, you get something that's autoreleased. As such, if you want it to live outside of your current scope you need to retain it.

Also, since NSString is not mutable, it would be better to use an NSMutableString for the "building" here.

I just realized you said "for school". I wrote something to do this that kept your same style, but won't post it since this is your homework. Basically I used NSMutableString for phone in randomPhone. I then used appendString: to append digits. I removed stringDigit completely, and just nested the stringWithFormat: inside the appendString:. Then I sent autorelease to phone before it was returned.

It's working fine for me, but doing it this way is... well, i guess not the way i'd do it. For one, what if the first three digits generated are X11? These numbers are generally reserved for emergencies, sort of emergencies, and information (at least in the US).

-Lee

 

[gnasher729]

Can we see the "randomNumberBetween" method?
And you have a memory leak when you initially create stringDigit.

 

[chown33]

I don't think your observation or the resulting diagnosis is correct.

If I define this:

[self randomNumberBetween:0 and:9];

to return ordinary sequential numbers (1, 2, 3, 4, etc.) then the output I see is:

rand = 123-456-7890

If you want to quickly check what's being built up, add this at the top of your for-loop:

NSLog( @"so far: %@", phone );

You should be able to see the progression as each part is appended.

If it shows all-1s being built, then show the code for the randomNumberBetween:and: method.

If it shows a correctly randomized number being built, then the problem is elsewhere, perhaps in whatever code is calling randomPhone. So show the code calling randomPhone.


For my test, I didn't bother correcting any of the leaks already mentioned. I simply tried to duplicate your described problem. All I did was define a random-digit function, which you didn't provide, and a suitable call to randomPhone that NSLog'ed the result.

There is a strikingly simple way to eliminate the leaks:

	NSString *phone = @"";
	NSString *stringDigit = @"";

This is not only correct, it avoids the needless alloc init verbosity. If it's unclear why this is correct, you need to revisit the Objective-C language documentation on what an @"" literal is. Hint: it's an actual object.

 

[isthisonetaken]

Ok, so first off this program is meant to build random data to store in a file so it can be bulk loaded into a database for a database course. We were told to just write something easy in C or C++, like 40 lines, but I wanted to do this as an exercise in Cocoa on the Mac as I'm trying to learn it for next year.

I've included this time all the specific parts of code that have been asked for. I added in the NSLog and it shows that it is only one number that is being built on the phone number.

Here is the code that calls randomPhone:

- (IBAction)generateAgents:(id)sender {
	numberOfAgents = [[agentsField stringValue] intValue];
	
	// run a for loop numberOfAgents times, each time creating a new random agent
	int id = 0;
	NSString *name = [[NSString alloc] init];
	NSString *phone = @"";
	NSString *email = [[NSString alloc] init];
	for (int i = 1; i <= numberOfAgents; i++) {
		id = i;
		name = [self randomName];
		phone = [self randomPhone];
		email = [self randomEmail];
		agentsData = [agentsData stringByAppendingString:phone];
		NSRunAlertPanel(@"", phone, NULL, NULL, NULL);
	}
	
	// write the contents of agentsData into the preview text field
	[filePreviewField setStringValue:agentsData];
}

This is the code that generates the random phone number:

- (NSString *)randomPhone {
	NSString *phone = @"";
	int digit;
	
	for (int i = 0; i < 12; i++) {
		//NSLog( @"so far: %@", phone );
		if (i == 3 || i == 7) {
			phone = [phone stringByAppendingString:@"-"];
		} else {
			digit = [self randomNumberBetween:0 and:9];
			phone = [phone stringByAppendingString:[NSString stringWithFormat:@"%d", digit]];
		}
	}
	
	return phone;
}

Finally, here is the code that returns a random number:

- (int)randomNumberBetween:(int)low and:(int)high {
	srand(time(NULL));
	
	return (rand() % high + low);
}

I like plinden's idea of only getting three random numbers so I tried doing that as well:

- (NSString *)randomPhone {
	int areaCode, phone1, phone2;

	areaCode = [self randomNumberBetween:100 and:999];
	phone1 = [self randomNumberBetween:100 and:999];
	phone2 = [self randomNumberBetween:1000 and:9999];
	NSString *phone = [[NSString alloc] initWithFormat:@"%d-%d-%d", areaCode, phone1, phone2];
	
	return phone;
}

This one is returning the same 3 digit combination, but a different 4 digit combination. Does that mean it's a problem with the random number generator?

Thanks for all the help so far guys!

Dan

 

[autorelease]

There is an issue with the random number generator.

You shouldn't be calling srand() every time. Since time() returns the current time in seconds, it's pretty much always going to be the case that all 10 calls to randomNumberBetween:and: will seed the random number generator with the same value, thus causing rand() to always return the same number.

Solution: remove srand(time(NULL)) from the random number method, and call it only once, when your program starts.

Also, there's another bug in randomNumberBetween:and: that occurs when "low" is greater than zero, but you should see if you can figure out what it is.

 

[gnasher729]

There is an issue with the random number generator.

You shouldn't be calling srand() every time. Since time() returns the current time in seconds, it's pretty much always going to be the case that all 10 calls to randomNumberBetween:and: will seed the random number generator with the same value, thus causing rand() to always return the same number.

Exactly what I thought because of the side effect: When the poster tried to debug his code, he always got different numbers, because more than a second was spent from one call to the next. When running the code at full speed, the time was always the same, and the random numbers were the same as well.

 

[isthisonetaken]

Holy crow guys! I think I got it now. I see what gnasher729 is saying about it working with breakpoints because it was seeding differently, but when no breakpoints it was always seeding the same thing. I moved that into applicationDidFinishLaunching. I think I found the bug for high low. I changed it to (high - low + 1) + low. I was basing the high + low on this http://www.cplusplus.com/reference/clibrary/cstdlib/rand/, but after some googling, I found this: http://www.cprogramming.com/tutorial/random.html

I also changed the actual randomPhone function to act like plinden mentioned.

Thanks for all the help guys!

Dan