1

So my attempt was that creating a program that automatically gets two matrixes' size from a .txt file and multiplies them. I could make the program with given sizes so in itself I only have problem with counting the cols and rows.

The input something like (MxN matrix):

1 2 3 4

1 2 3 4

1 2 3 4

To be specific, here is my program so far (the beginning of the code is relevant I think):

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

struct mat1
{
    int cols;
    int rows;
};

struct mat2
{
    int cols;
    int rows;
};

struct mat1 dim1(const char* file)
{
    struct mat1 m1;
    int rows = 0;
    int cols = 0;
    char c;
    FILE *f = fopen(file, "r+");

    while((c = fgetc(f) != EOF))
    {
        if(c != '\n' && rows == 0)
           {
            cols++;
           }
           else if(c == '\n')
            rows++;
    }
    rows++;
    return m1;
}

struct mat2 dim2(const char* file)
{
    struct mat2 m2;
    int rows = 0;
    int cols = 0;
    char c;
    FILE *f = fopen(file, "r+");

    while((c = fgetc(f) != EOF))
    {
        if(c != '\n' && rows == 0)
           {
            cols++;
           }
           else if(c == '\n')
            rows++;
    }
    rows++;
    return m2;
}


double* alloc_matrix(int cols, int rows) {
    double* m = (double*)malloc(cols * rows * sizeof(double));
    if (m == 0) {
        printf("Memory allocation error.\n");
        exit(-1);
    }
    return m;
}

void read_matrix(FILE* f, double* m, int cols, int rows) {
    for (int i = 0; i < rows; i++) {
        for (int j = 0; j < cols; j++) {
            fscanf(f, "%lf", &m[i * cols + j]);
        }
    }
}

void multiplication(double* m1, double* m2, double* m3, int cols, int rows) {
    for(int i = 0; i < rows; i++) {
        for(int j = 0; j < cols; j++) {
            m3[i * cols +j]=0;
            for(int k = 0; k < cols; k++) {
                m3[i * cols +j]+=m1[i * cols +k]*m2[k * cols +j];
            }
        }
    }
}

void write_matrix(double* m, int cols, int rows) {
    for (int i = 0; i < rows; i++) {
        for (int j = 0; j < cols; j++) {
            printf("%f ", m[i * cols + j]);
        }
        printf("\n");
    }
}


int main(int argc, char* argv[])
{
    char* matrix1 = argv[1];
    char* matrix2 = argv[2];

        if (argc < 3) {
        printf("Not enough arguments.\n");
        exit(-1);
    }

    struct mat1 m1 = dim1(matrix1);
    struct mat2 m2 = dim2(matrix2);
    printf(" %d   %d \n", m1.cols, m1.rows);
    printf(" %d   %d \n", m2.cols, m2.rows);
    int c1 = m1.cols;
    int r1 = m1.rows;
    int c2 = m2.cols;
    int r2 = m2.rows;

    if (r1!=c2)
    {
        printf("Matrixes are not suitable for multiplication. \n");
        exit(-1);
    }

    double* mtx1 = alloc_matrix(c1, r1);
    double* mtx2 = alloc_matrix(c2, r2);

    FILE* f1 = fopen(matrix1, "r");
    if (f1 == 0)
    {
        printf("Cannot open file %s.", argv[1]);
        exit(-1);
    }

    FILE* f2 = fopen(matrix2, "r");
    if (f1 == 0)
    {
        printf("Cannot open file %s.", argv[1]);
        exit(-1);
    }

    read_matrix(f1, mtx1, c1, r1);
    read_matrix(f2, mtx2, c2, r2);

    double* mtx3 = alloc_matrix(c1, r2);

    multiplication(mtx1, mtx2, mtx3, c1, r2);

    write_matrix(mtx3, c1, r2);

    free(mtx1);
    free(mtx2);
    free(mtx3);
    fclose(f1);
    fclose(f2);

    return 0;
}

When I tried it out with 2 3x3 matrixes, The outpot:

6422164 4199040 (from 2 printf()s that I set to check the dimensions).

6422164 4199040

Matrixes are not suitable for multiplication. (it's irrelevant)

So basically it doesn't use 3x3.

I cannot figure out what the problem is.

  • Don't post pictures of text. Post text. – Swordfish Dec 01 '18 at 22:30
  • @Swordfish Okay! Gonna edit it! – Levente Gáll Dec 01 '18 at 22:32
  • 2
    _Side note:_ No need to have _two_ `struct` definitions [since they are the same]. Use a single `struct mat` and then define `struct mat mat1;` and `struct mat mat2;`. The reason is that it doesn't scale (e.g. suppose you had 1000 matrixes, you'd do: `struct mat mat[1000];` rather than 1000 separate `struct` definitions). Also, note that `dim1()` and `dim2()` are identical except for the `struct` they operate on. So, create a single `dim()` function as well. – Craig Estey Dec 01 '18 at 22:35
  • @CraigEstey This was my first time to use struct for 2 returning variables and I wasn't sure! :) But thank you for letting me know! – Levente Gáll Dec 01 '18 at 22:39
  • 1
    Your column counting code is dubious. You show space separated numbers in the input, but the code in `dim1()` (and `dim2()`, but you're going to lose that shortly — Craig's comments are spot on!) counts each character in the first line as a column, which is going to cause trouble, sooner rather than later. – Jonathan Leffler Dec 01 '18 at 22:44
  • @JonathanLeffler How could I fix the counting part of it? Sorry If I ask obvious questions, I've been learning programming and C for about week. – Levente Gáll Dec 01 '18 at 22:51
  • Also, note that `dim1/dim2` "leak" `f` (i.e. no `fclose(f)`). So, add one at the bottom. And, `dim` could probably be combined with `read_matrix` if you added (e.g.) `double *m;` to the `struct` definition. (i.e.) `dim` gets the dimensions, does `mat.m = alloc_matrix(...);` and then does what `read_matrix` would do (after doing `rewind(f);`). Then, the struct has all the info you need instead of several scalar variables in `main`. – Craig Estey Dec 01 '18 at 22:54
  • 1
    There are many possibilities — I'm not sure which you'll think is best. One option is to read lines instead of characters. You can then scan the first line looking for white space between other characters; if the input includes the newline, then each field is terminated by a white space character, but there could be multiple consecutive white space characters between one pair of fields. You might [use `sscanf()` in a loop](https://stackoverflow.com/q/3975236/15168). _[…continued…]_ – Jonathan Leffler Dec 01 '18 at 22:56
  • 1
    _[…continuation…]_ Or you might continue with character input, skipping white space (`isspace()` from ``) and then counting sequences of non-spaces and spaces to determine the number of columns. There are other options too: for example, using `strtol()` and relatives on whole lines. you can use [`fgets()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/fgets.html) or POSIX [`getline()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/getline.html) to read lines. A good deal depends on how trustworthy your data sources are. – Jonathan Leffler Dec 01 '18 at 22:57
  • OT: regarding: `FILE *f = fopen(file, "r+");` always check (!=NULL) the returned value to assure the operation was successful – user3629249 Dec 01 '18 at 23:03
  • OT: regarding: `double* m = (double*)malloc(cols * rows * sizeof(double));` when calling any of the heap allocation function: `malloc` `calloc` `realloc` In C, the returned type is `void*` which can be assigned to any pointer. Casting just clutters the code, making it more difficult to understand, debug, etc. – user3629249 Dec 01 '18 at 23:07
  • OT: regarding: `printf("Cannot open file %s.", argv[1]);` error messages should be output to `stderr`, not `stdout`. and when the error indication is from a C library function, should also output the text reason the system thinks the error occurred. The function: `perror()` does all the above correctly; however, perror() does not handle replacement parameters, so use a buffer and `sprintf()` to format the error message before passing to `perror()` – user3629249 Dec 01 '18 at 23:09
  • You guys are fantastic! Feel like overwhelmed with the lot of informations you gave me. I'll examine it today! – Levente Gáll Dec 02 '18 at 07:54

1 Answers1

1

This is prefaced by my top comments.

I had to refactor dim to handle an arbitrarily large matrix, so I had to scan the first line of the file char-by-char, counting whitespace strings (which yields the number of columns - 1). It handles/strips leading/trailing whitespace [malformed]

I had dim then rewind the file and use fscanf and realloc to create the matrix dynamically.


Here's the working code [please pardon the gratuitous style cleanup]:

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

struct mat {
    int cols;
    int rows;
    double *m;
};

// size and read in matrix
struct mat
dim(const char *file)
{
    struct mat m;
    int rows = 0;
    int cols = 0;
    int maxcnt;
    int curcnt;
    int ret;
    int c;
    int c2;

    FILE *f = fopen(file, "r+");

    // strip leading whitespace [if any] off first line
    while (1) {
        c = fgetc(f);
        if (c == EOF)
            break;
        if (c == '\n')
            break;
        if (c != ' ')
            break;
    }

    // scan first line and count columns (number of space separaters)
    while (1) {
        c2 = ' ';
        while (1) {
            c = fgetc(f);
            if (c == EOF)
                break;

            if (c == '\n') {
                if (c2 != ' ')
                    ++cols;
                break;
            }

            if (c == ' ') {
                if (c != c2)
                    ++cols;
                break;
            }

            c2 = c;
        }

        if (c == '\n')
            break;
    }

    // convert number of whitespace separaters into number of columns
    if (cols > 0)
        ++cols;

    rewind(f);

    m.rows = 0;
    m.cols = cols;
    m.m = NULL;
    curcnt = 0;
    maxcnt = 0;

    while (1) {
        if (curcnt >= maxcnt) {
            maxcnt += m.cols * 100;
            double *tmp = realloc(m.m,sizeof(double) * maxcnt);
            if (tmp == NULL) {
                printf("dim: realloc failure\n");
                exit(1);
            }
            m.m = tmp;
        }

        ret = 0;
        for (int idx = 0;  idx < cols;  ++idx, ++curcnt) {
            ret = fscanf(f, "%lf", &m.m[curcnt]);
            if (ret != 1)
                break;
        }

        if (ret != 1)
            break;

        rows += 1;
    }

    fclose(f);

    m.rows = rows;

    // trim matrix to actual size;
    m.m = realloc(m.m,sizeof(double) * rows * cols);

    return m;
}

double *
alloc_matrix(int cols, int rows)
{
    double *m = (double *) malloc(cols * rows * sizeof(double));

    if (m == 0) {
        printf("Memory allocation error.\n");
        exit(-1);
    }
    return m;
}

void
multiplication(double *m1, double *m2, double *m3, int cols, int rows)
{
    for (int i = 0; i < rows; i++) {
        for (int j = 0; j < cols; j++) {
            m3[i * cols + j] = 0;
            for (int k = 0; k < cols; k++) {
                m3[i * cols + j] += m1[i * cols + k] * m2[k * cols + j];
            }
        }
    }
}

void
write_matrix(double *m, int cols, int rows)
{
    for (int i = 0; i < rows; i++) {
        for (int j = 0; j < cols; j++) {
            printf("%f ", m[i * cols + j]);
        }
        printf("\n");
    }
}

int
main(int argc, char *argv[])
{

    if (argc < 3) {
        printf("Not enough arguments.\n");
        exit(1);
    }

    struct mat m1 = dim(argv[1]);
    struct mat m2 = dim(argv[2]);

    printf(" %d   %d \n", m1.cols, m1.rows);
    printf(" %d   %d \n", m2.cols, m2.rows);

    int c1 = m1.cols;
    int r1 = m1.rows;
    int c2 = m2.cols;
    int r2 = m2.rows;

    if (r1 != c2) {
        printf("Matrixes are not suitable for multiplication.\n");
        exit(-1);
    }

    double *mtx3 = alloc_matrix(c1, r2);

    multiplication(m1.m, m2.m, mtx3, c1, r2);

    write_matrix(mtx3, c1, r2);

    free(m1.m);
    free(m2.m);
    free(mtx3);

    return 0;
}

Here are two test files I used. Note that although you can't see it, the first line has trailing whitespace [as a test]:

This is m1.txt:

 1 2  3  4
5 6 7  8
9 10  11  12

Here is the second file:

 1  2  3
 4 5 6
 7 8 9
 10 11 12

Here is the program output:

 4   3
 3   4
38.000000 44.000000 202.000000 232.000000
98.000000 116.000000 438.000000 504.000000
158.000000 188.000000 674.000000 776.000000
9.000000 10.000000 87.000000 100.000000

UPDATE:

Here's an alterate dim function that replaces the [somewhat fragile] char-by-char scan of the first line with a scan for newline [to get line length], followed by malloc of a buffer, fgets, and then loop on strtok to count the non-space strings in the lines (i.e. the number of columns):

// size and read in matrix
struct mat
dim(const char *file)
{
    struct mat m;
    int rows = 0;
    int cols = 0;
    int maxcnt;
    int curcnt;
    int ret;
    char *buf;
    char *bp;
    char *tok;
    int c;
    int c2;

    FILE *f = fopen(file, "r+");

    // count number of chars in first line of the file
    curcnt = 0;
    while (1) {
        c = fgetc(f);
        if (c == EOF)
            break;
        ++curcnt;
        if (c == '\n')
            break;
    }

    ++curcnt;
    buf = malloc(curcnt);
    rewind(f);
    fgets(buf,curcnt,f);

    cols = 0;
    bp = buf;
    while (1) {
        tok = strtok(bp," \n");
        if (tok == NULL)
            break;
        ++cols;
        bp = NULL;
    }
    free(buf);

    rewind(f);

    m.rows = 0;
    m.cols = cols;
    m.m = NULL;
    curcnt = 0;
    maxcnt = 0;

    while (1) {
        if (curcnt >= maxcnt) {
            maxcnt += m.cols * 100;
            double *tmp = realloc(m.m,sizeof(double) * maxcnt);
            if (tmp == NULL) {
                printf("dim: realloc failure\n");
                exit(1);
            }
            m.m = tmp;
        }

        ret = 0;
        for (int idx = 0;  idx < cols;  ++idx, ++curcnt) {
            ret = fscanf(f, "%lf", &m.m[curcnt]);
            if (ret != 1)
                break;
        }

        if (ret != 1)
            break;

        rows += 1;
    }

    fclose(f);

    m.rows = rows;

    // trim matrix to actual size;
    m.m = realloc(m.m,sizeof(double) * rows * cols);

    return m;
}

UPDATE #2:

I didn't like either solution for getting the number of columns, so here is a cleaner one that is as fast as the first one but is simpler and less cumbersome:

// scan first line and count columns
int
colcalc(FILE *f)
{
    int c;
    int noncur;
    int nonprev = 0;
    int cols = 0;

    while (1) {
        c = fgetc(f);
        if (c == EOF)
            break;

        if (c == '\n')
            break;

        // only count non-whitespace chars
        switch (c) {
        case ' ':
        case '\t':
            noncur = 0;
            break;
        default:
            noncur = 1;
            break;
        }

        // column starts on _first_ char in word
        if (noncur)
            cols += (noncur != nonprev);

        nonprev = noncur;
    }

    rewind(f);

    return cols;
}

UPDATE #3:

I tried out the previous 2 methods by you and it works so smoothly! Thank you once again! and your comments about making my program simpler with less variables and stuff!

You're welcome!

My coding style/methodology comes from a [very] old book: "The Elements of Programming Style" by Kernighan and Plauger.

The examples from that book are written in Fortran, but the maxims are on par with "Code Complete" by Steve McConnell.

From Chapter 7 [Efficiency and Instrumentation]:

  • Make it right before you make it faster.
  • Keep it right when you make it faster.
  • Make it clear before you make it faster.
  • Don't sacrifice clarity for small gains in "efficiency".
  • Don't strain to re-use code; reorganize instead.
  • Make sure special cases are truly special.
  • Keep it simple to make it faster.
  • Don't diddle code to make it faster -- find a better algorithm.
  • Instrument your programs. Measure before making "efficiency" changes.
Craig Estey
  • 30,627
  • 4
  • 24
  • 48
  • I tried out the previous 2 methods by you and it works so smoothly! Thank you once again! and your comments about making my program simpler with less variables and stuff! – Levente Gáll Dec 03 '18 at 00:26