Statistics mode function

Question

I will get straight to the point - I am a freshman at uni and I am currently working on my first bigger personal project - Statula is a terminal tool for data analysis which takes in dataset (as a file) and then spits out bunch of numbers about the numbers. I am quite satisfied with where it is going but I am not gonna lie - I have some concerns.

Here is the git repo.

I will say it beforehand in case someone goes for a lecture - I am aware that I did not check for returned errors in compute_dataset.
I have just reworked some of them and I am kind of busy this week - I will get onto it as soon as possible. Also, my tests suck - or rather a single one. I have just introduced new structure which basically broke all of my tests. I am also working on that.

So, back to the point.
My main focus is performance. I am satisfied with the fact that on my rig (i5-4670k) this program can finish it's job in under 240ms. However, that is not good enough for me. I believe that the main performance hog might be the mode function - and for a good reason.

Here is the code:

int mode(struct dataset *set)
{
    assert(set->number_count>0);
    double max_value=0;
    int max_count=0,i,j,mode_count=1;
    for(i=0;i<set->number_count;i++){
        int count=0;
        for(j=i+1;set->numbers[j]==set->numbers[i]&&j<set->number_count;j++);
        count=j-i;
        if(count>max_count){
            mode_count=1;
            max_count=count;
            max_value=set->numbers[i];
        } else if(count==max_count) {
            mode_count++;
            i=j-1;
        }
    }
    if(mode_count==1){
        *(set->mode)=max_value; 
        set->is_mode_present=1;
    } else{
        *(set->mode) = 0;
        set->is_mode_present=0;
    }
    return mode_count==1?1:0;
}

Perhaps there is some room for improvement. For the record, the number set is qsorted before being passed to the function. That's the main premise. Can it get any faster than that?

I also wonder whether the struct itself is a good idea - I guess there is no going back. Do not get me wrong, I have put a lot of thought into this matter and I came to a conclusion that by using struct I can:

1. Analyse multiple sets by reading multiple datasets - each has it's own struct. Beforehand, all I had was a local variable.

2. Avoid some unneccessary function calls - for instance, I have needed mean to compute standard deviation. Sure, I could pass it as an argument to a function, but.. where is the fun?

Unfortunately, that means that you cannot just copy-paste the function out of the source code and expect it to work - it relies on dataset structure being present.

So that is more of a general question - do you prefer your code to be correct (in a very loose sense) or portable?

General code suggestions are also welcome!

Answer 1

Well, of course your code is too slow, it's O(n²) but should be O(n):
Unless you already found a run of the same length, you don't skip to the end of the run!

Your inner for-loop only tests whether you are beyond your list after already having read there. That's slightly too late, rearrange that.

A side-question: Why do you insist on the dataset not being empty?

Why do you call your mode max_value? That's strange.

Restrict the scope of your values to the minimum you need: If it's not in scope, there's nothing for you to keep track of, allowing you to concentrate on other issues.

Consider investing in a bit more whitespace, especially around binary operators.

You know that true is 1 and false is 0?

Never use ; as the empty statement: It always looks suspicious. Either put a comment before it /**/; or use a block {} instead.