0

For my homework it is given one dimensional array and i have to convert it in a two dimensional array. The two dimensional array has 2 for the number of columns, because i have to represent the one dimensional array as pairs(the value of the number, the number of appearences in the array). This is what have tried. The error appears on the last 2 lines of code: access violation writing location 0xfdfdfdfd.

#include <iostream>
#include <stdlib.h>
using namespace std;

int main()
{
    const int NR=17;
    int arr[NR]={6,7,3,1,3,2,4,4,7,5,1,1,5,6,6,4,5};
    int **newArr;
    int count=0;
    int countLines=0;
    int searched;
    for(int i=0;i<NR;i++)
    {
            newArr=new int*[countLines];
        for(int i=0;i<countLines;i++)
        {
            newArr[i]=new int[2];
        }
        searched=arr[i];
        if(i>0)
        {
            for(int k=0;k<countLines;k++)
            {
                if(newArr[countLines][0] == searched)
                {
                    searched=arr[i]++;
                }

                for(int j=0;j<NR;j++)
                {
                    if(searched==arr[j])
                    {
                        count++;
                    }
                }
                countLines++;
            }
        }
        else
        {
            for(int j=0;j<NR;j++)
            {
                if(searched==arr[j])
                {
                    count++;
                }
            }
            countLines++;
        }

        newArr[countLines][0]=searched;
        newArr[countLines][1]=count;
    }
}
3
  • You are using newArrin the first loop without allocating it any memory. Commented Nov 17, 2012 at 13:34
  • 1
    Would strongly suggest that you debug thru the code and inspect the values of the variables at the point of failure. Thats the best way to learn and figure out whats going wrong. With respect to the write failure, remember that array index is zero based. So if you allocate an array of size "countLines", then you cannot access newArr[countLines]. You need to say newArr[countLines-1]. Commented Nov 17, 2012 at 13:40
  • Even after fixing the seg-fault, this code is wrong at the top of its voice. How about the memory leak with each iteration of the outter loop (blasting over the old newArr pointer with a fresh allocation). As written this will allocate NR pointer arrays, leaking the old one with each iteration. Commented Nov 17, 2012 at 13:54

2 Answers 2

3

First you are using newArr in the first loop before allocating it any memory. You cannot dereference a pointer which owns no legal memory. It results in undefined behavior.

Secondly in the last part, you are allocating newArr a memory equal to countLines thus.

newArr = new int*[countLines] ;

It means that the indices in the first dimension of newArr are 0------>countLines-1. Doing newArr[countLines][0] = searched ; is again undefined. Make it newArr[countLines - 1].

Sign up to request clarification or add additional context in comments.

2 Comments

Apparently these were the errors. Otherwise, I would also suggest as @Raam suggested: Debug your code line by line.
You have made the changes but you have not done it right. Look at the logic of your code, look at what it actually does, line by line. To give one example for(int i=0;i<NR;i++) { newArr=new int*[countLines];. You have put your array allocation inside the loop. This means you will allocate your array 17 times. Did you want to do that? I don't think so, you want to allocate it only once. In programming you have to understand every last detail of what your code does, and when you have bugs understand why the code you wrote is different from the code you wanted to write.
1

I'm not going to bother with a line-by-line code analysis since (a) you're changing it while people are answering your question and (b) it would literally take too long. But here's a summary (non-exhaustive) of klunkers:

  1. You are leaking memory (newArr) on each loop iteration starting with the second.
  2. You're out-of-bounds on your array access multiple times.
  3. You should not need to use a pointer array at all to solve this. A single array of dimension [N][2] where N is the number of unique values.

One (of countless many) way you can solve this problem is presented below:

#include <iostream>
#include <algorithm>

int main()
{
    // 0. Declare array and length
    int arr[]={6,7,3,1,3,2,4,4,7,5,1,1,5,6,6,4,5};
    const size_t NR = sizeof(arr)/sizeof(arr[0]);

    // 1. sort the input array
    std::sort(arr, arr+NR);

    /* alternaive sort. for this input size bubble-sort is
       more than adequate, in case your limited to not being
       allowed to use the standard library sort */
    /*
    for (size_t i=0;i<NR;++i)
        for (size_t j=i+1;j<NR;++j)
            if (arr[i] > arr[j])
            {
                arr[i] ^= arr[j];
                arr[j] ^= arr[i];
                arr[i] ^= arr[j];
            }
    */

    // 2. single scan to determine distinct values
    size_t unique = 1;
    for (size_t i=1;i<NR;++i)
        if (arr[i] != arr[i-1])
            unique++;

    // 3. Allocate a [unique][2] array
    int (*newArr)[2] = new int[unique][2];

    // 4. Walk array once more, accumulating counts
    size_t j=0;
    newArr[j][0] = arr[0];
    newArr[j][1] = 1;
    for (size_t i=1;i<NR;++i)
    {
        if (arr[i] != arr[i-1])
        {
            newArr[++j][0] = arr[i];
            newArr[j][1] = 0;
        }
        ++newArr[j][1];
    }

    // 5. Dump output
    for (size_t i=0;i<unique;++i)
        cout << newArr[i][0] << " : " << newArr[i][1] << endl;

    delete [] newArr;

    return EXIT_SUCCESS;
}

Output

1 : 3
2 : 1
3 : 2
4 : 3
5 : 3
6 : 3
7 : 2

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.