0

I have this code written for multiplying matrixes. It works but when I try to put it inside it's own function it stops working. Why does it do that?

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

/* Ask for elements to insert inside matrixes.*/
void AddValues(int row, int col, float **ptr, int matrixNumber) {
    printf("\nWrite elements to insert into %d. matrix :\n", matrixNumber);
    for (int i = 0; i < row; i++) {
        for (int j = 0; j < col; j++) {
            printf("\tA[%d][%d] = ", i, j);
            scanf("%f", &ptr[i][j]);
        }
    }
}

void Multiply(int row1, int col1, int col2, float **ptr1, float **ptr2, float **ptr3) {
    /* Multiplication. */
    for (int i = 0; i < row1; i++)  {
        for (int j = 0; j < col1; j++) {
            ptr3[i][j] = 0;
            for (int k = 0; k < col2; k++)
                ptr3[i][j] = ptr3[i][j] + ptr1[i][k] * ptr2[k][j];
        }
    }
}

void WriteOut(int row1, int col2, float **ptr3) {
    /* Print final matrix. */
    printf("\n\nFinal Matrix :");
    for (int i = 0; i < row1; i++) {
        printf("\n\t\t\t");
        for (int j = 0; j < col2; j++)
            printf("%f\t", ptr3[i][j]);
    }
}

void HighestSum(int row1, int col2, float **ptr3) {
    printf("\n\nrow with highest sum of elements :");
    float max = 0;
    float sum;
    for (int i = 0; i < row1; ++i) {
        for (int j = 0; j < col2; ++j){
            sum = sum + ptr3[i][j];
            if (j == col2 - 1)
                printf("\n");
        }
        if (max == 0)
            max = sum;
        if (max < sum)
            max = sum;
        sum = 0;
    }
    printf("Highest sum of elements in row: %f  ", max);
}

int main() {
    float **ptr1, **ptr2, **ptr3;
    int row1, col1, row2, col2;
    int i;
    printf("\nWrite numbers of rows for first matrix. : ");
    scanf("%d", &row1);
    printf("\nWrite numbers of columns for first matrix. : ");
    scanf("%d", &col1);
    printf("\nWrite numbers of rows for second matrix. : ");
    scanf("%d", &row2);
    printf("\nWrite numbers of columns for second matrix. : ");
    scanf("%d", &col2);
    if (col1 != row2) {
        printf("\nMatrixes can't be multiplied.");
        return (0);
    }
    if (col1 <= 0 || row1 <= 0 || col2 <= 0 || row2 <= 0) {
        printf("\nNumbers have to be positive.");
        return (0);
    }
    /* Allocation. */
    ptr1 = (float **)malloc(sizeof(float *) * row1);
    ptr2 = (float **)malloc(sizeof(float *) * row2);
    ptr3 = (float **)malloc(sizeof(float *) * row1);
    for (i = 0; i < row1; i++)
        ptr1[i] = (float *)malloc(sizeof(float) * col1);
    for (i = 0; i < row2; i++)
        ptr2[i] = (float *)malloc(sizeof(float) * col2);
    for (i = 0; i < row1; i++)
        ptr3[i] = (float *)malloc(sizeof(float) * col2);
    AddValues(row1, col1, ptr1, 1);
    AddValues(row2, col2, ptr2, 2);
    Multiply(row1, col1, col2, ptr1, ptr2, ptr3);
    WriteOut(row1, col2, ptr3);
    HighestSum(row1, col2, ptr3);
    return 0;
}

Program written like this works. However when I put scanf in its own function like this it doesn't

void Scanner(int row1, int col1, int row2, int col2) {
    printf("\nWrite numbers of rows for first matrix. : ");
    scanf("%d", &row1);
    printf("\nWrite numbers of columns for first matrix. : ");
    scanf("%d", &col1);
    printf("\nWrite numbers of rows for second matrix. : ");
    scanf("%d", &row2);
    printf("\nWrite numbers of columns for second matrix. : ");
    scanf("%d", &col2);
    if (col1 != row2) {
        printf("\nMatrixes can't be multiplied.");
        return (0);
    }
    if (col1 <= 0 || row1 <= 0 || col2 <= 0 || row2 <= 0) {
        printf("\nNumbers have to be positive.");
        return (0);
    }
}

The program will ask for size of rows and cols but then it stops. How can I make it work?

chqrlie
  • 131,814
  • 10
  • 121
  • 189
kpcttobee
  • 67
  • 5
  • 2
    Make a [mre], nobody wants to read irrelevant code. – Cheatah May 05 '22 at 19:57
  • 2
    Your `Scanner()` function takes its parameters by value... also, **always** check the return value of `scanf()` or you have uninitialized values / undefined behavior waiting to happen. – DevSolar May 05 '22 at 19:59
  • The problem is almost certainly variable scope. Whatever happens in a function to parameters or local variables, has no effect outside of the function. You should either pass pointers to integers as function parameters, or return all results in something like a struct. – Cheatah May 05 '22 at 20:00
  • 2
    `Scanner` operates on _copies_ of its parameters, so calls to `scanf` inside `Scanner` will modify these copies, not the original variables. BTW, this is the reason why you need to pass pointers like `&row1` to `scanf` itself - because you want `scanf` to _modify `row1`_. – ForceBru May 05 '22 at 20:00
  • When you `scanf` a `variable`, you need to pass `&variable`, for example `scanf("%d", &row1);` rather than `scanf("%d", row1);`. **Why do you think this is the case?** Could `scanf` be written without this silly requirement? If you know the answer, apply it to your own function. – n. m. could be an AI May 05 '22 at 20:23

1 Answers1

2

In the function Scanner, the parameters are passed by value. This means that the function Scanner will get its own copy of these variables, and changing the values of these copies will not change the values of the original variables in the function main.

If you want the function Scanner to be able to change the values of the original variables in the function main, then you should not pass the values of the original variables, but you should instead pass pointers that point to the original variables in the function main. That way, the function Scanner can use the pointers to write directly to the original variables.

I suggest that you change the function Scanner to the following:

void Scanner( int *p_row1, int *p_col1, int *p_row2, int *p_col2 )
{
    printf( "Write numbers of rows for first matrix: " );
    scanf( "%d", p_row1 );
    printf( "Write numbers of columns for first matrix: " );
    scanf( "%d", p_col1 );
    printf( "Write numbers of rows for second matrix: " );
    scanf( "%d", p_row2 );
    printf( "Write numbers of columns for second matrix: " );
    scanf( "%d", p_col2 );

    if( *p_col1 != *p_row2 )
    {
        printf( "Matrixes can't be multiplied.\n" );
        exit( EXIT_FAILURE );
    }

    if( *p_col1 <= 0 || *p_row1 <= 0 || *p_col2 <= 0 || *p_row2 <= 0 )
    {
        printf( "Numbers have to be positive.\n" );
        exit( EXIT_FAILURE );
    }
}

The prefix p_ that I added to the variable names just stands for "pointer", but you don't have to use that prefix. You can give them any name that you want.

You can call the function Scanner from the function main like this:

Scanner( &row1, &col1, &row2, &col2 );

Also, since you are dealing with user input, it would probably be better to always check the return value of scanf, in order to determine whether the function succeeded. Otherwise, if the user enters something that is not a number, the conversion will fail, but your program will still attempt to process the non-existant converted value.

Therefore, it would probably be better to write the function like this:

void Scanner( int *p_row1, int *p_col1, int *p_row2, int *p_col2 )
{
    printf( "Write numbers of rows for first matrix: " );
    if ( scanf( "%d", p_row1 ) != 1 )
    {
        printf( "Input error!\n" );
        exit( EXIT_FAILURE );
    }

    printf( "Write numbers of columns for first matrix: " );
    if ( scanf( "%d", p_col1 ) != 1 )
    {
        printf( "Input error!\n" );
        exit( EXIT_FAILURE );
    }

    printf( "Write numbers of rows for second matrix: " );
    if ( scanf( "%d", p_row2 ) != 1 )
    {
        printf( "Input error!\n" );
        exit( EXIT_FAILURE );
    }

    printf( "Write numbers of columns for second matrix: " );
    if ( scanf( "%d", p_col2 ) != 1 )
    {
        printf( "Input error!\n" );
        exit( EXIT_FAILURE );
    }

    if( *p_col1 != *p_row2 )
    {
        printf( "Matrixes can't be multiplied.\n" );
        exit( EXIT_FAILURE );
    }

    if( *p_col1 <= 0 || *p_row1 <= 0 || *p_col2 <= 0 || *p_row2 <= 0 )
    {
        printf( "Numbers have to be positive.\n" );
        exit( EXIT_FAILURE );
    }
}

However, this solution still does not perform very good input validation yet. For example, if the user enters the number 6abc, then scanf will accept this input as valid input for the number 6. If you do not want this, then you may want to use the function get_int_from_user from this answer of mine to another question. That function will perform very strict input validation, and it will automatically reprompt the user for input if the input is not a number.

If you do decide to use my function get_int_from_user, then you could write the function like this:

void Scanner( int *p_row1, int *p_col1, int *p_row2, int *p_col2 )
{
    *p_row1 = get_int_from_user(
        "Write numbers of rows for first matrix: "
    );

    *p_col1 = get_int_from_user(
        "Write numbers of columns for first matrix: "
    );

    *p_row2 = get_int_from_user(
        "Write numbers of rows for second matrix: "
    );

    *p_col2 = get_int_from_user(
        "Write numbers of columns for second matrix: "
    );

    if( *p_col1 != *p_row2 )
    {
        printf( "Matrixes can't be multiplied.\n" );
        exit( EXIT_FAILURE );
    }

    if( *p_col1 <= 0 || *p_row1 <= 0 || *p_col2 <= 0 || *p_row2 <= 0 )
    {
        printf( "Numbers have to be positive.\n" );
        exit( EXIT_FAILURE );
    }
}

However, the user is now automatically reprompted by the function get_int_from_user when the input is not a number, but the user won't get reprompted at all when the input is invalid for another reason. To fix this inconsistency, the function could be written like this:

void Scanner( int *p_row1, int *p_col1, int *p_row2, int *p_col2 )
{
    //loop forever until input is valid
    for (;;) //infinite loop, equivalent to while(1)
    {
        *p_row1 = get_int_from_user(
            "Write numbers of rows for first matrix: "
        );

        *p_col1 = get_int_from_user(
            "Write numbers of columns for first matrix: "
        );

        *p_row2 = get_int_from_user(
            "Write numbers of rows for second matrix: "
        );

        *p_col2 = get_int_from_user(
            "Write numbers of columns for second matrix: "
        );

        if( *p_col1 != *p_row2 )
        {
            printf( "Matrixes can't be multiplied. Try again!\n\n" );
            continue;
        }

        if( *p_col1 <= 0 || *p_row1 <= 0 || *p_col2 <= 0 || *p_row2 <= 0 )
        {
            printf( "Numbers have to be positive.\n\n" );
            continue;
        }

        //input is valid, so we can break out of the infinite loop
        break;
    }
}

This program could be improved further by restarting the input as soon as the user enters one non-positive number, instead of waiting until the user has entered all 4 numbers. However, I don't want to make my answer to complicated.

Here is a small demonstration program of the latest version of the function Scanner:

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

void Scanner( int *p_row1, int *p_col1, int *p_row2, int *p_col2 );
int get_int_from_user( const char *prompt );

int main( void )
{
    int row1, col1, row2, col2;

    Scanner( &row1, &col1, &row2, &col2 );

    printf(
        "\n"
        "The variables in the function main now have the\n"
        "following values:\n"
        "\n"
        "row1: %d\n"
        "col1: %d\n"
        "row2: %d\n"
        "col2: %d\n",
        row1, col1, row2, col2
    );
}

void Scanner( int *p_row1, int *p_col1, int *p_row2, int *p_col2 )
{
    //loop forever until input is valid
    for (;;) //infinite loop, equivalent to while(1)
    {
        *p_row1 = get_int_from_user(
            "Write numbers of rows for first matrix: "
        );

        *p_col1 = get_int_from_user(
            "Write numbers of columns for first matrix: "
        );

        *p_row2 = get_int_from_user(
            "Write numbers of rows for second matrix: "
        );

        *p_col2 = get_int_from_user(
            "Write numbers of columns for second matrix: "
        );

        if( *p_col1 != *p_row2 )
        {
            printf( "Matrixes can't be multiplied. Try again!\n\n" );
            continue;
        }

        if( *p_col1 <= 0 || *p_row1 <= 0 || *p_col2 <= 0 || *p_row2 <= 0 )
        {
            printf( "Numbers have to be positive.\n\n" );
            continue;
        }

        //input is valid, so we can break out of the infinite loop
        break;
    }
}

int get_int_from_user( const char *prompt )
{
    for (;;) //loop forever until user enters a valid number
    {
        char buffer[1024], *p;
        long l;

        //prompt user for input
        fputs( prompt, stdout );

        //get one line of input from input stream
        if ( fgets( buffer, sizeof buffer, stdin ) == NULL )
        {
            fprintf( stderr, "unrecoverable error reading from input!\n" );
            exit( EXIT_FAILURE );
        }

        //make sure that entire line was read in (i.e. that
        //the buffer was not too small)
        if ( strchr( buffer, '\n' ) == NULL && !feof( stdin ) )
        {
            int c;

            printf( "line input was too long!\n" );

            //discard remainder of line
            do
            {
                c = getchar();

                if ( c == EOF )
                {
                    fprintf( stderr, "unrecoverable error reading from input!\n" );
                    exit( EXIT_FAILURE );
                }

            } while ( c != '\n' );

            continue;
        }

        //attempt to convert string to number
        errno = 0;
        l = strtol( buffer, &p, 10 );
        if ( p == buffer )
        {
            printf( "error converting string to number!\n" );
            continue;
        }

        //make sure that number is representable as an "int"
        if ( errno == ERANGE || l < INT_MIN || l > INT_MAX )
        {
            printf( "number out of range error!\n" );
            continue;
        }

        //make sure that remainder of line contains only whitespace,
        //so that input such as "6sdfj23jlj" gets rejected
        for ( ; *p != '\0'; p++ )
        {
            if ( !isspace( (unsigned char)*p ) )
            {
                printf( "unexpected input encountered!\n" );

                //cannot use `continue` here, because that would go to
                //the next iteration of the innermost loop, but we
                //want to go to the next iteration of the outer loop
                goto continue_outer_loop;
            }
        }

        return l;

    continue_outer_loop:
        continue;
    }
}

This program has the following behavior

Write numbers of rows for first matrix: abc
error converting string to number!
Write numbers of rows for first matrix: 6abc
unexpected input encountered!
Write numbers of rows for first matrix: 6
Write numbers of columns for first matrix: 7
Write numbers of rows for second matrix: 8
Write numbers of columns for second matrix: 9
Matrixes can't be multiplied. Try again!

Write numbers of rows for first matrix: 6
Write numbers of columns for first matrix: 7
Write numbers of rows for second matrix: 7
Write numbers of columns for second matrix: 8

The variables in the function main now have the
following values:

row1: 6
col1: 7
row2: 7
col2: 8
Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
  • I changed it to return; because the function is void and the answer works perfectly thank you very much ! – kpcttobee May 05 '22 at 20:57