Return to Revisions

Presentation

Code formatted and presented well. Refreshing. Compiled properly - almost.

Initialization

With median_init(), the queue is pre-populated with the value of 0. This implies that the return values of min,median,max are all bogus until n values added. it would make sense to either

1. Allow an initialization value: median_init(...., float init_value)

2. Or better, functions on min,median,max as if it had less than n elements are in it until n elements are added.

Complexity

Instead of the structures involved, this task could be solved with less data usage: a length, eldest index and a single array as below and still maintain O(n) complexity. Adding is a simple as scanning for the eldest index in a sorted array, replacing it with the new value. Then bubble sort the new value into place.

typedef struct MedfiltDataAlt {
    float value;
    size_t queue_order;
} MedfiltDataAlt;

Allocation responsibility

This code obliges the calling code to provide the space needed. This fine. Yet I find it more useful for the median_...() functions to handle that. OP's choice on this, just wanted to provide an alternative POV.

Added functionality

A means to report the min,median,max without adding a new number would be useful.

Parameter order

I'd expect the state data first.

// void median(float input, MedfiltData *data, float *median, float *min, float *max)
   void median(MedfiltData *data, float input, float *median, float *min, float *max)

NAN

As float can take on a value of not-a-number. Code's use of > and < does not clearly push the NAN is a proscribed manner. When NANs are involved, > is not the opposite of <=. If code was amended to deal with a growing array during the first n calls of median(), consider a median(NAN, ...) to have the effect of shortening the queue and not saving the NAN.

Naming

The .c file (assume medfilt.c to match medfilt.h) name, the type names and function names would be more uniform if they all began with the same prefix and case rather than Medfilt, median and medfilt.

This is manifested in the minor code error in main.c

// medfilt_init(&data, nodes, KERNEL);
median_init(&data, nodes, KERNEL);

Portability

#pragma once is not defined in the C standard

Function at 0

Minor: Both median() and median_init() fail with a size of 0. I would expect code to tolerate this edge case and not cause undefined behavior.

display()

Minor: I'd expect code to use "%zu" to match size_t or a cast to a wide unsigned type to maintain same sign-ness. Also int min, int mid, int max --> float min, float mid, float max and use "%g" or "%e". A goal of display is diagnostic. It should avoid data truncation.

// printf("u[%d] =...", (int)i, ...);
printf("u[%zu] =...", i, ...);
printf("u[%lu] =...", (unsigned long)i, ...);