3

The following code gets input from a file and stores it in a 1D array. I want a matrix type of input like:

1,2,4
3,4,5
5,6,7

(or)

2,3,4,5
4,5,6,7
7,6,5,4
3,4,5,6

where the size of the matrix varies and they are separated by commas, to be stored in a 2D array. What changes should I make to the following code?

#include <stdio.h>
#include <stdlib.h>

int main(){
    char file[51];
    int data, row, col, c, count, inc;
    int *array, capacity=50;
    char ch;
    array = (int*)malloc(sizeof(int) * capacity);
    printf("\nEnter the name of the file with its extention\n");
    scanf("%s", file);
    FILE *fp = fopen(file, "r"); 
    row = col = c = count = 0;
    while (EOF != (inc = fscanf(fp,"%d%c", &data, &ch)) && inc == 2){
        ++c; //COLUMN count
        if (capacity == count)
            array = (int*)realloc(array, sizeof(int) * (capacity *= 2));
        array[count++] = data;
        if(ch == '\n'){
            ++row;
            if (col == 0){
                col = c;
            } else if (col != c){
                fprintf(stderr, "format error of different Column of Row at %d\n", row);
                goto exit;
            }
            c = 0;
        } else if (ch != ',') {
            fprintf(stderr, "format error of different separator(%c) of Row at %d \n", ch, row);
            goto exit;
        }
    }
    {   //check print
        int i, j;
        //int (*matrix)[col] = array;
        for(i = 0; i < row; ++i){
            for(j = 0; j < col; ++j)
                printf("%d ", array[i * col + j]);//matrix[i][j]
            printf("\n");
        }
    }
exit:
    fclose(fp);
    free(array);
    return 0;
}
Raj
  • 83
  • 1
  • 10
  • 4
    They say [you shouldn't cast the result of `malloc()` in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – MikeCAT Mar 04 '16 at 10:21
  • 1
    One of changes to make: add error checking to memory allocation and file open. – MikeCAT Mar 04 '16 at 10:22
  • Thank you for that. Changes noted. – Raj Mar 04 '16 at 10:23
  • 1
    `goto` directives should always be avoided by using conditions. – muXXmit2X Mar 04 '16 at 10:27
  • @muXXmit2X: There are few good use-cases for `goto`. This could be one. – too honest for this site Mar 04 '16 at 11:36
  • 1
    @Olaf Yes I know. However since `goto` can be very prone to errors it shouldn't be used if it is not 100% neccessary. And usually it isn't and can be avoided. E.g. in this case one could also write a function `cleanup(FILE* fp, int* array)` which frees the allocated memory and closes the file. – muXXmit2X Mar 04 '16 at 11:54
  • @muXXmit2X: As you can **always** avoid `goto`, your comment is redundant. It is a matter of readability. `goto` with a clear lable name (like here) and jumping only forward can be better to read/understand. backward `goto` (i.e. generating loops) is a no-go(to), though. – too honest for this site Mar 04 '16 at 12:03
  • Is using `goto` so old use of the language? – Ed Heal Mar 06 '16 at 14:32

3 Answers3

1

This uses fgets to read each line and strtol to parse integers from a line.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <limits.h>

int get_int_range ( char *line, char **next, char *term, int *value, int min, int max);
int get_int_series ( int cols, int dest[][cols], int inputrow, int min, int max, char *line, char *delim);
int get_int_count ( int min, int max, char *line, char *delim);

int main( int argc, char *argv[])
{
    char line[900] = {'\0'};
    char file[100] = {'\0'};
    int valid = 0;
    int rows = 0;
    int cols = 0;
    int eachrow = 0;
    int eachcol = 0;
    FILE *fp = NULL;

    printf ( "Enter the name of the file with it's extension\n");
    fgets ( file, sizeof ( file), stdin);
    file[strcspn ( file, "\n")] = '\0';//remove trailing newline

    if ( ( fp = fopen ( file, "r")) != NULL) {

        fgets ( line, sizeof ( line), fp);//read a line
        rows = get_int_count ( INT_MIN, INT_MAX, line, ",\n");
        rewind ( fp);
        if ( rows) {
            cols = rows;
            //once the size is obtained, the array can be declared
            int array[rows][cols];

            for(eachrow = 0; eachrow < rows; eachrow++) {
                if ( ( fgets ( line, sizeof ( line), fp)) == NULL) {//read a line
                    fclose ( fp);
                    printf ( "Problem! not enough lines in file\n");
                    return 1;
                }
                valid = get_int_series ( cols, array, eachrow, INT_MIN, INT_MAX, line, ", \n");
                if ( !valid) {
                    fclose ( fp);
                    printf ( "Problem!\n");
                    return 1;
                }
            }
            if ( ( fgets ( line, sizeof ( line), fp)) != NULL) {//read a line
                fclose ( fp);
                printf ( "Problem! too many lines in file\n");
                return 1;
            }
            for(eachrow = 0; eachrow < rows; eachrow++) {
                for(eachcol = 0; eachcol < cols; eachcol++) {
                    printf("[%d] ", array[eachrow][eachcol]);
                }
                printf("\n");
            }
            printf("\nDone\n");
        }
        fclose ( fp);
    }
    return 0;
}

int get_int_range ( char *line, char **next, char *term, int *value, int min, int max)
{
    long int input = 0;
    char *end = NULL;

    errno = 0;
    input = strtol ( line, &end, 10);//get the integer from the line
    if ( end == line) {
        printf ( "input MUST be a number\n");
        return 0;
    }
    if ( *end != '\0' && ( strchr ( term, *end) == NULL)) {
        printf ( "problem with input: [%s] \n", line);
        return 0;
    }
    if ( ( errno == ERANGE && ( input == LONG_MAX || input == LONG_MIN))
    || ( errno != 0 && input == 0)){
        perror ( "input");
        return 0;
    }
    if ( input < min || input > max) {
        printf ( "input out of range %d to %d\n", min, max);
        return 0;
    }

    if ( next != NULL) {
        *next = end;
    }
    *value = input;//set the value
    return 1;
}

int get_int_series ( int cols, int dest[][cols], int inputrow, int min, int max, char *line, char *delim)
{
    char *end = NULL;
    char *each = NULL;
    int valid = 0;
    int input = 0;
    int count = 0;
    int temp[cols];

    each = line;
    do {
        valid = get_int_range ( each, &end, delim, &input, INT_MIN, INT_MAX);
        if ( !valid) {
            printf ( "input MUST be a number\n");
            return 0;
        }
        if ( valid) {
            temp[count] = input;
            count++;
            if ( count > cols) {
                printf ( "too many integers. %d entered. only enter %d\n", count, cols);
                return 0;
            }
        }
        while ( *end && strchr ( delim, *end)) {//skip any number of delimitors
            end++;
        }
        each = end;
    } while ( end && *end);

    if ( count < cols) {
        printf ( "too few integers. need %d entered. only entered %d\n", cols, count);
        return 0;
    }
    while ( count) {
        count--;
        dest[inputrow][count] = temp[count];//set the value
    }
    return 1;
}

int get_int_count ( int min, int max, char *line, char *delim)
{
    char *end = NULL;
    char *each = NULL;
    int valid = 0;
    int input = 0;
    int count = 0;

    each = line;
    do {
        valid = get_int_range ( each, &end, delim, &input, INT_MIN, INT_MAX);
        if ( !valid) {
            return count;
        }
        if ( valid) {
            count++;
        }
        while ( *end && strchr ( delim, *end)) {//skip any number of delimitors
            end++;
        }
        each = end;
    } while ( end && *end);

    return count;
}
user3121023
  • 8,181
  • 5
  • 18
  • 16
0

Simply save and read the array always a single dimension string which length is the product of all dimensions by the sizeof the type. Then return a pointer to void that will be assigned to a pointer to an array of desired dimensions.
I.e.

void *MyReadFnc(char *filename, size_t size)
{
    char *p = malloc(size);
    //open file and load the data
    return (void *)p;
}
...
//Call the function to read in our array.
//For who don't see it we want assign to an array of 5 dimensions
//array[10][2][3][4][5]
//Note that the first dimension is omitted, but we have to use it in array size calcullation
int (*array)[2][3][4][5] = MyReadFnc("Filename.ext", 2*3*4*5*10*sizeof(int));
...
printf("element[9][1][2][3][4] = %d\n", array[9][1][2][3][4]);
...
//Again an array of 3 dimensions:
//array2[10][20][30]
int (*array2)[20][30] = MyReadFnc("Filename.ext", 20*30*10*sizeof(int));
...
printf("element[9][19][29] = %d\n", array[9][19][29]);

We use a void * as output, because the routine is generic and can handle whichever array (of any number of dimensions). And the use of void * is simply the standard and natural way of C language to handle such variegated types. This is the way C language works. If you wants stronger type checking you should change language.
The following is more generic code for reading arrays.

#include <stdio.h>
#include <stdlib.h>

//Because we read whole data as single stream like a 1 dimension array, totElems
//is the total number of elements expressed as the product of all dimensions of
//the final array
void *ReadArray(char *fname, size_t totElems, size_t ElemSize)
{
    FILE *fp = fopen(fname, "r");
    if (!fp)
    {
        fprintf(stderr, "Can't open file <%s>\n", fname);
        exit(-1);
    }

    int *p = calloc(totElems, ElemSize);
    if (!p)
    {
        fprintf(stderr, "Can't allocate memory\n");
        exit(-1);
    }

    for (size_t i=0; ; i++)
    {
        int a;

        if (EOF==fscanf(fp, "%d,", &a))
            break;
        p[i] = a;
    }

    fclose(fp);
    return (void *)p;
}

int main(void)
{
    char file[51];
    printf("\nEnter the name of the file with its extention\n");
    scanf("%s", file);

    printf("Array 4x4\n");
    //First sample: declare a pointer to a 2 dimension array 4x4 elements
    int (*array)[4];
    array = ReadArray(file, 16, sizeof(int));
    for (int i=0; i<4; i++)
    {
        for(int j=0; j<4; j++)
            printf("%d,", array[i][j]);
        printf("\n");
    }

    printf("Array 2x8\n");
    //Second sample: declare a pointer to a 2 dimension array 2x8 elements
    int (*array2)[8];
    array2 = ReadArray(file, 16, sizeof(int));
    for (int i=0; i<2; i++)
    {
        for(int j=0; j<8; j++)
            printf("%d,", array2[i][j]);
        printf("\n");
    }

    printf("Array 2x2x4\n");
    //Third sample: declare a pointer to a 3 dimension array 2x2x4 elements
    int (*array3)[2][4];
    array3 = ReadArray(file, 16, sizeof(int));
    for (int i=0; i<2; i++)
    {
        for(int j=0; j<2; j++)
        {
            if(j) printf (" - ");
            for (int k=0; k<4; k++)
                printf("%d,", array3[i][j][k]);
        }
        printf("\n");
    }

    return 0;
}

You can assign the result to arrays of any dimensions ;). No casting is required because the array object is returned as a void * to allow maximum flexibility. Note also that because any C array is an array of array of array, etc, it is really dimensions independent. What is returned by the function is an array that could fit any definition respecting 2 claims: 1. the array is of a type having the same sizeof specified in ElemSize (that here for sake of simplicity is hardwired to int), 2. the product of whole dimensions is <=totElems.
This of course is just a sample to serve as base to develop more complex versions, a good development could be to make it able to handle any type of data (not only int). I'm sure that this is a good exercise for beginners that needs a starting point to use to improve themselves and expand their creativity.
Finally just consider that on a C99-C11 compliant compiler you can write: printf("Array 2x2x4 using variables\n");

//Fourth sample: declare a pointer to a 3 dimension array 2x2x4 elements
//using variables for dimensions
int x = 2;
int y = 2;
int z = 4;
int (*array4)[y][z];
array4 = ReadArray(file, 16, sizeof(int));
for (int i=0; i<x; i++)
{
    for(int j=0; j<y; j++)
    {
        if(j) printf (" - ");
        for (int k=0; k<z; k++)
            printf("%d,", array4[i][j][k]);
    }
    printf("\n");
}
Frankie_C
  • 4,764
  • 1
  • 13
  • 30
  • 1) Don't use magic numbers. They are a guarantee for trouble 2) A straight forward approach is to always use the `sizeof` the object the pointer points to: `sizeof(*array)`: No redundancy and you don't have to think about the correct type (you still have tgo multiply by the outer dimension, of course). – too honest for this site Mar 04 '16 at 11:39
  • See Wikipedia: https://en.wikipedia.org/wiki/Magic_number_(programming)#Unnamed_numerical_constants – too honest for this site Mar 04 '16 at 12:00
  • @Olaf still don't understand. Those numbers are the arbitrary dimensions of the user array not magic numbers. There are no dangling numbers around the only one that was referenced to something related to compiler is the `sizeof(int)`. – Frankie_C Mar 04 '16 at 12:19
  • That was no joke. And yes, they are magic numbers. The linked article is quite clear. I'd really liked to give an UV for using true n-D arrays, but breaking type system without need is a bad approach. Why use an error-prone `void *` and _magic numbers_ instead of macro-constants and the actual type and other flaws? – too honest for this site Mar 04 '16 at 13:14
  • @Olaf Once again Olaf, maybe I'm confused, but *I don't understand where are the magic numbers you refer to?*. Could you be so kind to show me what you exactly mean? – Frankie_C Mar 06 '16 at 00:44
0

Ok, your present code already knows how to determine the number of cols at the end of first line and to control that all lines has same number of cols.

You can allocate a 2D array at the end of the first line that way:

capacity = cols * cols;
int (* arr)[cols] = malloc(sizeof(int) * capacity); // array is a 2D array cols x cols

(of course, you must have stored first line in another array, and you must copy those data in newly allocate arr...)

Then you assign that way:

arr[row][c] = data;

The only requirement is that your compiler accepts Variable Length Arrays. It is specified in C99 but some old MS compilers (at least up to 2008) do not accept it.

Serge Ballesta
  • 143,923
  • 11
  • 122
  • 252