4
\$\begingroup\$

The task is to provide a function that returns the earliest time point when a worker is allowed to start duty.

The function uses two parameters:

  • Time stamp when the rest starts

  • The number of local nights which is necessary according to a table

I wonder if there is anything wrong with void convertTimeStructToLocalTime(TimeStamp * const timeStructToConvert) in the code. Is it OK to use constant pointer to the TimeStamp structure in convertTimeStructToLocalTime?

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

//has tm_isdst flag to be used for daylight saving time
typedef struct tm TimeStamp;
typedef enum {false, true} bool;

TimeStamp getEndOfRest(TimeStamp startOfRest, int neededLocalNights);
bool isNeededLocalNightsInTheRange(int neededLocalNights);
void convertTimeStructToLocalTime(TimeStamp * const timeStructToConvert);

//to make tests easier
void wrapGetEndOfRest(TimeStamp startOfRest, int neededLocalNights);

int main(void) {
    int y=2016, m=2, d=27;
    int y2=2016, m2=12, d2=28;
    int daysDelta = 0;

    TimeStamp t = { .tm_year=y-1900, .tm_mon=m-1, .tm_mday=d, .tm_hour=12, .tm_min=5, .tm_sec=10 };
    TimeStamp t2 = { .tm_year=y2-1900, .tm_mon=m2-1, .tm_mday=d2, .tm_hour=9, .tm_min=59, .tm_sec=5};

    //tests
    for(daysDelta = -1; daysDelta < 8; daysDelta++){
        convertTimeStructToLocalTime(&t);
        printf("%s is the start add %d days\n", asctime(&t), daysDelta);
        wrapGetEndOfRest(t, daysDelta);
    }

    for(daysDelta = -1; daysDelta < 8; daysDelta++){
        convertTimeStructToLocalTime(&t2);
        printf("%s is the start add %d days\n", asctime(&t2), daysDelta);
        wrapGetEndOfRest(t2, daysDelta);
    }

    return EXIT_SUCCESS;
}


//Since the minimum for neededLocalNights is 2 according to the table the time of departure is always 6 am
TimeStamp getEndOfRest(TimeStamp startOfRest, int neededLocalNights){
    startOfRest.tm_mday += neededLocalNights;
    startOfRest.tm_hour = 6;
    startOfRest.tm_min = 0;
    startOfRest.tm_sec = 0;
    convertTimeStructToLocalTime(&startOfRest);
    return startOfRest;
}

bool isNeededLocalNightsInTheRange(int neededLocalNights){
    if(2 <= neededLocalNights && neededLocalNights <= 5){
        return true;
    }
    else{
        return false;
    }
}

void convertTimeStructToLocalTime(TimeStamp * const timeStructToConvert){
    int ret = mktime(timeStructToConvert);
    if( ret == -1 ){
        printf("Error: unable to make time using mktime\n");
    }
}

void wrapGetEndOfRest(TimeStamp startOfRest, int neededLocalNights){
    if(isNeededLocalNightsInTheRange(neededLocalNights)){
        startOfRest = getEndOfRest(startOfRest, neededLocalNights);
        printf("%s is the end\n\n", asctime(&startOfRest));
    }
    else{
        printf("Error: the neededLocalNights is out of range min 2, max 5 got: %d \n\n", neededLocalNights);
    }
}
\$\endgroup\$
2
\$\begingroup\$
  • Why not just use <stdbool.h> instead of defining your own enum for it? If it has to do with the standard being used, then make sure you're compiling under C99 or GNU99.

  • It looks like t and t2 are for testing and more could be added at some point. If so, you could have an array of TimeStamps to help make the code more concise.

  • isNeededLocalNightsInTheRange() can be simplified as such:

    return 2 <= neededLocalNights && neededLocalNights <= 5;
    

    This will automatically return the appropriate condition.

  • Try to avoid magic numbers by instead making them into constants. This would apply to the 2, 8, 6, 5, and 0 (the -1 indicates an error and doesn't apply here). Comments aren't always helpful enough and don't explain each of these anyway.

  • Error messages should be printed to fprintf() with its first argument as stderr to indicate this. printf only prints to standard output.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.