Problem copying random numbers

[alexchen]

I cannot copy random numbers without repeating in a map.
Unhandled exception at 0x0040115e in file.exe: 0xC0000005: Access violation writing location 0xabababaf.

int **map;

int randomize(int numberToSelect,int range,int counter){



int* remainingNumbers = new int[range];
int* chosenNumbers = new int[numberToSelect];
 

for(int i = 0; i < range; i++)   
  remainingNumbers[i] = i;
 
for(int i = 0; i < numberToSelect; i++)
  {
    const int selectedElement = rand() % (range - i);

    chosenNumbers[i]=remainingNumbers[selectedElement] + 1; 
  

    remainingNumbers[selectedElement]=remainingNumbers[range-i];
}
delete [] remainingNumbers; 


for(int i = 0; i < counter; i++)return chosenNumbers[i];


delete [] chosenNumbers;

}
bool Memalloc(int Width,int Height)
{

		map = new int*[Width];

		for( int x=0;x<Width;x++){
	
		map[x] = new int[Height];
		}
		
 
return true;
}
void create(int Width, int Height){

	Memalloc(Width,Height);
	int Area,x,y,balance;
	Area = Width * Height;
	x = 1, y = 0,balance = 0;

	for(int i = 0; i < Area; i++){
		if(y < i - balance)y++;
		 if(y ==Height)x+=1,balance = y,y=1;
		
			map[x][y] = randomize(Area, Area,i);
	}


	delete [] map;

}

[VernonDozier]

The formatting makes the code almost impossible to read, at least for me.

Line 54 - Are you intentionally using commas or are those supposed to be semicolons? Could be OK, but looks a bit suspicious. I'm more than a bit rusty on commas and comma operators, if I ever understood where to put them in the first place. It just looks out of place there.

Line 26 - Why have a loop? This code can execute at most one time since there's a return statement.

Line 29 - Unreachable if choice >= 1.

[VernonDozier]

Line 29 - Unreachable if choice >= 1.

Edit:

Line 29 - Unreachable if counter >= 1.

[alexchen]

Something is wrong with my code. I corrected Every mistake that you found. But it is still not working.Please, solve it.

int **map;
int randomize(int numberToSelect,int range){

int* remainingNumbers = new int[range];
int* chosenNumbers = new int[numberToSelect];
 

for(int i = 0; i < range; i++)   
  remainingNumbers[i] = i;
 
for(int i = 0; i < numberToSelect; i++)
  {
    const int selectedElement = rand() % (range - i);

    chosenNumbers[i]=remainingNumbers[selectedElement] + 1; 

    remainingNumbers[selectedElement]=remainingNumbers[range-i];
}
delete [] remainingNumbers; 

return *chosenNumbers;

}
bool Memalloc(int Width,int Height)
{

		map = new int*[Width];

		for( int x=0;x<Width;x++){
	
		map[x] = new int[Height];
		}
		
 
return true;
}
void create(int Width, int Height){

	Memalloc(Width,Height);
	int Area;
	Area = Width * Height;

	for(int x = 0; x < Width; x++){
		for(int y = 0; y < Height; y++){

			map[x][y] = randomize(Area, Area);
			cout<<map[x][y];
		}
		cout<<endl;
	}
	}

[VernonDozier]

>> I corrected Every mistake that you found.

No, you have not. Your code is unreadable. That is the MAIN problem. No one can follow your code with the formatting. Not me, not even you. Formatting is crucial. read this link.

http://www.gidnetwork.com/b-38.html

Visual Studio and NetBeans both have auto formatters.

>> Please, solve it

That's not the way it works. You need to explain the problem and your algorithm in words, explain exactly what you're trying to do, explain where it's going wrong, ask a specific question, and then we guide you towards the answer, but you need to express a willingness to do the lion's share of the work.

At first glance, assuming that I understand what "randomize" is supposed to do (i.e. shuffle an array?), I don't know why you'd want to allocate any memory in a function like that. The memory should already be allocated. You're just shuffling. Can't see any reason to allocate dynamic memory there. Some comments are needed to describe what the intent of the function is.

[alexchen]

Thanks for helping me. I make changes in my random number generator. This time it paste the numbers in an array. The error might be located inside the function create.

Width = 5
Height = 2
Is my code formatted?

void randomize(int numberToSelect,int range, int *list){

int* remainingNumbers = new int[range];
int* chosenNumbers = new int[numberToSelect];
 
list = new int[numberToSelect];

for(int i = 0; i < range; i++)   
  remainingNumbers[i] = i;
 
for(int i = 0; i < numberToSelect; i++)
  {
    const int selectedElement = rand() % (range - i);

    chosenNumbers[i]=remainingNumbers[selectedElement] + 1; 

    remainingNumbers[selectedElement]=remainingNumbers[range-i];
}
delete [] remainingNumbers; 

for(int i = 0; i < range; i++) list[i] = chosenNumbers[i];

delete [] chosenNumbers;

}

void create(int Width, int Height){

	Memalloc(Width, Height);
	int Area;
	Area = Width * Height;
        //The error might be here
	randomize(Area, Area, number);
	for(int i = 0; i < 10; i++) cout << number[i] <<endl;

	}

[VernonDozier]

This is your code formatted. Note the:

1. Brackets so it is obvious where nested code stasrts and stops.
2. Lack of tabs. All tabs are replace by four spaces.
3. Constant indentation. All nested code is indented four spaces in.
4. Blank lines. Extra blank lines are deleted. Blank lines are also added to group code.

void randomize(int numberToSelect,int range, int *list)
{
    int* remainingNumbers = new int[range];
    int* chosenNumbers = new int[numberToSelect];

    list = new int[numberToSelect];

    for(int i = 0; i < range; i++)
    {
        remainingNumbers[i] = i;
    }

    for(int i = 0; i < numberToSelect; i++)
    {
        const int selectedElement = rand() % (range - i);
        chosenNumbers[i]=remainingNumbers[selectedElement] + 1; 
        remainingNumbers[selectedElement]=remainingNumbers[range-i];
    }

    delete []remainingNumbers; 

    for(int i = 0; i < range; i++) 
    {
        list[i] = chosenNumbers[i];
    }

    delete []chosenNumbers;
}


void create(int Width, int Height)
{
    Memalloc(Width, Height);

    int Area;
    Area = Width * Height;

    //The error might be here
    randomize(Area, Area, number);
    for(int i = 0; i < 10; i++)
    {
        cout << number[i] <<endl;
    }
}

What happened to the Memalloc function? What calls the create function? It isn't posted anymore. Don't make us go back several posts to grab code. Post so we can do a quick copy/paste.

I'm not sure what the randomize function does. You should add comments. What does the function do and what do the parameters represent. It's not obvious. I can't get into what might be a solution because I have no idea what you're trying to do with this function. Line 6 looks like a memory leak and it looks like list is a local pointer that goes out of scope, then you try to use it.

Allocate the memory BEFORE you call the function and pass the array to it. Don't allocate the memory in the function.

But it's hard to say. We don't know where number is defined. We don't know what calls the create function.

[alexchen]

Thanks for your help. I don`t know how to copy the random numbers in the map.
Can you give me a hint. Here is my code:

void randomize(int numberToSelect,int range, int *list){

int* remainingNumbers = new int[range];
int* chosenNumbers = new int[numberToSelect];
 
//create an array
for(int i = 0; i < range; i++)
{
	remainingNumbers[i] = i;
}
//randomize
for(int i = 0; i < numberToSelect; i++)
  {
    const int selectedElement = rand() % (range - i);
    chosenNumbers[i]=remainingNumbers[selectedElement] + 1; 
    remainingNumbers[selectedElement]=remainingNumbers[range-i];
}
delete [] remainingNumbers; 

//copy the random numbers without repeating.
for(int i = 0; i < range; i++)
{
list[i] = chosenNumbers[i];
}

delete [] chosenNumbers;

}
bool Memalloc(int Width,int Height)
{
		map = new int*[Width];

		for( int x=0;x<Width;x++)
		{
		map[x] = new int[Height];
		}

return true;
}
void create(int Width, int Height){
//First, it allocate the map.
	Memalloc(Width,Height);
	int Area, *number;
	Area = Width * Height;
	number = new int[Area];

	randomize(Area,Area,number);
	for(int i = 0; i < 10; i++)
	{
		cout << number[i] << endl;
	//How can I copy this list in my map?
	}
}

[VernonDozier]

Your code got unformatted again. Use my code's formatting as a guide.

You want to be really careful with terms like Width and Height. Generally, it would be this:

bool Memalloc(int Width,int Height)
{
    map = new int*[Height];
 
    for( int x=0;x<Height;x++)
    {
        map[x] = new int[Width];
    }
 
    return true;
}

not this:

bool Memalloc(int Width,int Height)
{
    map = new int*[Width];
 
    for( int x=0;x<Width;x++)
    {
        map[x] = new int[Height];
    }
 
    return true;
}

You'll have an array for each row, not each column.

But moving on with the way you originally defined the terms...

Copying will be the same regardless of whether it's random numbers. Memory is memory. You figure out what index corresponds to what index and copy.

If you want to copy an array, you can loop through the arrays and copy an element at a time. You'll need to make a transformation to get from the 1 dimensional array to the 2-dimensional array.

int* map;
int* number;

// code to allocate any memory and fill in number array goes here.  Presumably this is done already in lines 45 and 47.

// copy Width times Height integers below from list to map.
for (int i = 0; i < Height; i++)
{
    for (int j = 0; j < Width; j++)
    {
        int index = /* calculate corresponding number index here based on i and j */
        map[i][j] = number[index];
    }
}

Consider using names other than "map" and "list" for you arrays. C++ has both of them. Anyone reading your code could get confused when you use those names if you aren't careful.

[VernonDozier]

Edit. 30 minute clock expired, so adding new post:

map is an int** type, not an int* type. Change line 1 above to this:

int** map;

[alexchen]

Part of the numbers can be pasted on the map. I don´t know how to calculate the index.
Please help me.

void create(int Width, int Height){

	Memalloc(Width,Height);
	int Area, *number;
	Area = Width * Height;
	number = new int[Area];

	randomize(Area,Area,number);

for (int i = 0; i < Height; i++)
{
     for (int j = 0; j < Width; j++)
	{
		int index = j + ((i - 1) * Width);//the error might be here
		map[i][j] = number[index];
		cout << map[i][j] << " ";
	}
	cout << endl;
}
	}

[VernonDozier]

Time for some debugging then. Display index to the screen after you calculate it. Make sure it's right. If it's negative, it's wrong.

Pencil and paper first. Do the math. You need a 1 to 1 mapping between the two arrays. Then implement it in C++.

And you need a much more specific question for people to help.

[alexchen]

I am sure it is not a negative number. Thanks for the advice.

[VernonDozier]

I am sure it is not a negative number. Thanks for the advice.

If you're sure, you shouldn't be. Try printing it out anyway. index is negative. Look at the code again.

for (int i = 0; i < Height; i++)
{
  for (int j = 0; j < Width; j++)
  {
    int index = j + ((i - 1) * Width);//the error might be here
    map[i][j] = number[index];
    cout << map[i][j] << " ";
  }
  cout << endl;
}

[alexchen]

You`re right. Thanks for your help.

void create(int Width, int Height){

	Memalloc(Width,Height);
	int Area, *number;
	Area = Width * Height;
	number = new int[Area];

	randomize(Area,Area,number);
for (int i = 0; i < Height; i++)
{
	for (int j = 0; j < Width; j++)
	{
		int index = j + i * Width;
		map[i][j] = number[index];
		cout << map[i][j] << " ";
	}
	cout << endl;
}
	}

[alexchen]

I cannot solve it. Maybe the index is wrong.

int index = j + i * Width;