#help with creating a max function for arrays of doubles?

82 messages · Page 1 of 1 (latest)

proud oriole
# 
double maxim(double* arr, int size) {
    double* ptr = arr;
    double max = *ptr; 
    for (int i = 1; i < size; i++) {
        if (*(ptr + i) > max) { 
            max = *(ptr + i);
        }
    }
    return max;
}

it seems like it only takes in the first value of the array instead of its entirety, im not sure if its because of the way i wrote the function or rather the way i wrote my list into it later in the main.

next scrollBOT
#

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

naive shuttle
#

It looks fine to me. How are you calling it?

proud oriole
exotic mist
# proud oriole ``` double maxim(double* arr, int size) { double* ptr = arr; double max ...

Imo

ptr[i]
```is much more readable than 
```c
*(ptr + i)
```but both do the same.

You could also just completely drop the `ptr` and replace it by simply using `arr` instead.

Otherwise the function looks good (unless of course for the edge case where `size = 0` in which case you would have an out of bounds access in the line `double max = *ptr` (which is the same as `double max = *(ptr + 0)` which is the same as `double max = ptr[0]`))
proud oriole
exotic mist
proud oriole
#

in short i created a struct of Course with some fields, which one of the fields is the struct Students which has inside of it the Fgrade for any different student

exotic mist
#

example values

proud oriole
# 
                       ID = 8915
                       Fgrade = 84.6 }
exotic mist
#

So... Fgrade is just a single number?

proud oriole
#

and student2 and so on will have different values

exotic mist
#

And what is course1.studentNum?

proud oriole
#

its an int of the number of students in the course

#

wait it just occured to me, Fgrade isnt saved as an array, it's just many structs i want to read the value of Fgrade

exotic mist
#

Okay. Now let's think about this.

You pass your maxim function - which is able to find the maximum value out of an array of numbers - a single number but you tell it that's actually an array with as many numbers as there are students (which is obviously more than the single number you just gave it as a first parameter)

proud oriole
#

so that means reading the Fgrade of the i-th student will look something like that course1.student1[i]->Fgrade right?

exotic mist
#

student1 isn't an array

#

So no, that won't work

#

From what I see so far and from what you need it appears as if your program is majorly missstructured

proud oriole
#

can i sent here the entire code, i dont know how to trim it down so its more than 200 lines long

exotic mist
#

Sure, let's see if we can spot anything immediately obvious

proud oriole
#

i do understand that my functions of max and min are good but not for my use case, unless i create an array with the Fgrades of all the students

exotic mist
#

shiver scanf(" %[^\n]", buffer) shiver

#

That just screams buffer overflow

exotic mist
#

That's not even a member of your Course struct

#

But yeah, what you'd need to pass is either just the Course struct and you can then grab the relevant pieces inside the function, or you pass course1.students and course1.studentNum as the both parameters and keep the function almost as it is

#

Also instead of malloc(strlen...) and strcpy you could just use strdup

#

!man strdup

next scrollBOT
#
man7
strdup(3)

strdup, strndup, strdupa, strndupa - duplicate a string

Synopsis
#include <string.h>

char *strdup(const char *s);

char *strndup(const char *s, size_t n);
char *strdupa(const char *s);
char *strndupa(const char *s, size_t n);

Feature Test Macro Requirements for glibc (see feature_test_macros(7)):

strdup():
    _XOPEN_SOURCE >= 500
        || /* Since glibc 2.12: */ _POSIX_C_SOURCE >= 200809L

... (truncated)

proud oriole
nimble island
proud oriole
exotic mist
#

I mean, obviously yeah. But the latter option requires less reworking (although I personally think the former is cleaner)

proud oriole
#

and if i want to pass course1.students as you suggested i dont understand it at all

#

how will i be able to access and compare all the Fgrade values?

exotic mist
# 
double max_fgrade(Course course);  // this one passes a copy 
```or 
```c
double max_fgrade(Course *course);  // this one also passes a copy but a copy of the pointer, therefore pointing to the original element

The latter way is generally a bit more performant while the former way can make your intent clearer (i.e. you don't want to alter course in any way, shape or form so you pass it only a copy, this way it can't be altered even if you were to try it inside your max_fgrade function

#

And then you just iterate through all course.students

#

and from each of those students you iterate over you check the fgrade

proud oriole
# exotic mist ```c double max_fgrade(Course course); // this one passes a copy ```or ```c d...

sorry but i dont understand how it should work, i tried with the pointer

double maxim(Course* course, int size) {
    double* ptr = course;
    double max = *ptr; 
    for (int i = 1; i <= size; i++) {
        if (*(ptr + i) > max) { 
            max = *(ptr + i);
        }
    }
    return max;
}

double maxGrade = maxim(&course1.students->Fgrade, course1.studentNum);

how exactly do i access the Fgrade for all the students this way? if i do it this way searchs the single Fgrade each time

exotic mist
#

You want to iterate over all students.

Also what are you even doing in this code?
You have a pointer course to Course and you convert it to a double pointer which has completely different properties than the actual Course struct.
This can't work.
You actually should scrap the implementation entirely as I think it's only confusing you and start fresh.
How about you start with just a loop over all the students in a course. Just the loop, no actual implementation yet and we'll go from there

proud oriole
exotic mist
proud oriole
#

i thought about a (maybe) simpler solution imo, in the loop where i add students and calculate the final grade for each stedent ive done this

#

this is the loop of the initialization of students as well as finding each Fgrade

exotic mist
#

course->students + course->studentNum
That's just out of bounds when you dereference it and therefore UB

proud oriole
exotic mist
#

Also the code isn't logically correct.
You're just checking the first's and one too last's (i.e. the out of bounds access) grades

proud oriole
# 
//finding the max final grade
            *maxFgrade = (((course->students + course->studentNum)->Fgrade) > ((course->students)->Fgrade)) ? (course->students + course->studentNum)->Fgrade : (course->students)->Fgrade;
            printf("the max grade for the first %d students is %lf\n", course->studentNum, *maxFgrade);
            
            //finding the min final grade
            *minFgrade = (((course->students + course->studentNum)->Fgrade) < ((course->students)->Fgrade)) ? (course->students + course->studentNum)->Fgrade : (course->students)->Fgrade;
            printf("the min grade for the first %d students is %lf\n", course->studentNum, *minFgrade);
exotic mist
#

Also the code is really hard to read (and probably to maintain, too)

proud oriole
# exotic mist Also the code is really hard to read (and probably to maintain, too)

wait isnt the following right?

//finding the max final grade
            *maxFgrade = (((course->students + course->studentNum -1 )->Fgrade) > (*maxFgrade) ? (course->students + course->studentNum - 1)->Fgrade : (course->students)->Fgrade;
            printf("the max grade for the first %d students is %lf\n", course->studentNum, *maxFgrade);
            
            //finding the min final grade
            *minFgrade = (((course->students + course->studentNum - 1)->Fgrade) < (*minFgrade) ? (course->students + course->studentNum - 1)->Fgrade : (course->students)->Fgrade;
            printf("the min grade for the first %d students is %lf\n", course->studentNum, *minFgrade);
exotic mist
#

This fixed the UB since it fixes the out of bounds access, but now the only two fgrades you're comparing are of the first and last student, i.e. your code can produce incorrect results if there are more than 2 students in a course

#

Also you should really use easier to read notation, e.g. instead of

(((course->students + course->studentNum -1 )->Fgrade)
```you could write:
```c
course->students[course->studentNum - 1].Fgrade
```which still looks atrocious but at least is a bit more readable

But yeah, that's just an example, as I said, that code there is logically incorrect
proud oriole
exotic mist
#

What your big complicated line basically says is:

maxFgrade = fgrade_of_last_student > fgrade_of_first_student ? fgrade_of_last_student : fgrade_of_first_student;
```(assuming that `maxFgrade` was - as you said - initialized to be the fgrade of the first student).

See, you're only respecting the two students, but not the 2nd, 3rd, 4th, etc
#

You even say it yourself:

if true then that means this student Fgrade is the max between the two

proud oriole
#

it should always hold, no?

exotic mist
#

Oh, I see.
Oh.
That's so awkward to read.
I guess it can work, I mean does it work?
You probably need to put *maxFgrade at the end of the ternary for the false case.

But this is such awkward code since your main function is just a clusterfuck of utility, i.e. it's doing everything. There's no nice separation into functions that are easily testable apart from each other but everything's interleaved. Such code is really annoying to test and verify.

proud oriole
exotic mist
#

Oh, and the course->studentNum - 1 wasn't actually the correct call since you only increment the value of course->studentNum at the very end of your function.

proud oriole
#

this is inside the create course func

exotic mist
#

Why would you need to calculate the max grades in the create function?

#

That's like completely separate topics

proud oriole
#

my problem with this is that it says that both pointers maxFgrade and minFgrade are not initialized and i dont understand why it says so when i initilized them to the course->student->Fgrade

exotic mist
#

ah no, that's such bad code design.
That's like stuffing your spaghetti bolognaise and chocolate cake together.

proud oriole
#

and also i somehow need to access both of these at the main too

exotic mist
#

They're just pointers, but they don't point to any valid memory as of now, meaning you can't store any values there

#

You'd need to malloc (and eventually free) them, but why not just use a simple normal variable that isn't a pointer?

proud oriole
exotic mist
#

It clicks eventually and suddenly it's the most obvious thing

proud oriole
exotic mist
#

But you're not even creating them in the main

proud oriole
proud oriole