How to make it more beautiful?

Question

Example code from the application for the Pebble watch. The code works and outputs a timer between two timestamp, but how to make it more beautiful?

char d[5];
char h[5];
char m[5];
char s[5];
const char *start = "START";

time_t now_time = time(NULL);
int tt = other_time - now_time;

if (tt > 0) {
    int rest = tt % 31556926;
    int dd = rest / 86400;
    rest = tt % 86400;
    int hh = rest / 3600;
    rest = tt % 3600;
    int mm = rest / 60;
    int ss = rest % 60;

    if (dd > 0) 
        snprintf(d, sizeof(d), "%dD", dd);
    else
        snprintf(d, sizeof(d), "%s", "\n");
    ...
    if (ss > 0)
        snprintf(s, sizeof(s), " %dS", ss);
    else
        snprintf(s, sizeof(s), "%s", "\n");

    snprintf(string, sizeof(string), "%s%s%s%s", d, h, m, s);  
} else {
    snprintf(string, sizeof(string), "%s", start);
}

What's "beautiful" really? Anyway, if you want to ask for [codereview.se], there's a site on the network for that. — StoryTeller - Unslander Monica, Jan 11 '18 at 12:30
_Fashionize_ it ;) Or tell us what exactly you mean by "beautiful" — B001ᛦ, Jan 11 '18 at 12:31
Beauty is in the eye of the beholder but one improvement you could make would be to replace the hard coded numbers with const declarations that indicate what the values mean, e.g. what does `31556926` mean? — Kerri Brown, Jan 11 '18 at 12:36
@aspire89 So in your code it would be good to declare `static const int SecondsPerYear = 31556926`. Makes it more understandable for people reading your code. — Kerri Brown, Jan 11 '18 at 12:49
Putting the code in a function to make it valid C code would help a lot to make it more beautiful. — Gerhardh, Jan 11 '18 at 12:50
Actually, what you have posted does not even compile (as is). But some suggestions: 1) change `d,h,m,s, tt` to `days`, `hours`, `minutes`, `seconds`, `totalTime` (or `duration`) . 2) use curly braces `{...}` liberally. 3) pay attention to indents to keep code easily readable. Although the terms readability and maintainability may not equate to beauty, when it comes to code, they are close enough. — ryyker, Jan 11 '18 at 13:17
I would make actual constants (e.g. 3600 second in hour) a const. I would also use ?: operator instead of if anything > 0 / else — Vel, Jan 11 '18 at 13:46

Jango · Accepted Answer · 2019-02-06T16:50:05.803

If by "beautiful" you mean "elegant" (which is usually taken as "concise and efficient"), I would try something like:

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

char * getDuration(time_t future)
{
    char * duration = (char *) malloc(sizeof(char) * 32);
    int elapsed = (int) (future - time(NULL));
    if(elapsed > 0)
    {
        int d = (elapsed % 31556926) / 86400;
        int h = (elapsed %    86400) /  3600;
        int m = (elapsed %     3600) /    60;
        int s = (elapsed %       60)        ;
        sprintf(duration, "%sD%sH%sM%sS", d, h, m, s);  
    }
    else sprintf(duration, "START");
    return duration;
}

PS: I didn't get why you were printing "\n" in the strings instead of "0D", "0H" etc.

[What are the functions from the standard library that must/should be avoided?](https://stackoverflow.com/questions/2565727/what-are-the-functions-from-the-standard-library-that-must-should-be-avoided) Don't teach strncpy to newbies. — Lundin, Jan 11 '18 at 14:09

How to make it more beautiful?

1 Answers1