[PATCH RESEND] size reduction and coding style for who.c

Devin Bayer devin at freeshell.org
Sat Mar 11 21:56:31 UTC 2006


On Mar 11, 2006, at 12:07, Tito wrote:

> On Saturday 11 March 2006 19:24, Rob wrote:
>
>> Changing spaces to tabs is cool.  Why the extra curly brackets  
>> around single
>> line ifs?  (I haven't rejected this one, I'm just asking if there's a
>> rationale here.  Seems an aesthetic choice.)
>
> 	    if (stat(name, &st) == 0) {
> 		now = time(NULL);
> 		idle = now -  st.st_atime;
>
> 		if (idle < 60)
> 		    printf("00:00m    ");
> 		else if (idle < (60 * 60))
> 		    printf("00:%02dm    ", (int)(idle / 60));
> 		else if (idle < (24 * 60 * 60))
> 		    printf("%02d:%02dm    ", (int)(idle / (60 * 60)),
> 			   (int)(idle % (60 * 60)) / 60);
> 		else if (idle < (24 * 60 * 60 * 365))
> 		    printf("%03ddays   ", (int)(idle / (24 * 60 * 60)));
> 		else
> 		    printf("%02dyears   ", (int) (idle / (24 * 60 * 60 * 365)));
> 	    } else
> 		printf("%-8s  ", "?");
>
> 	    printf("%-12.12s   %s\n", ctime((time_t*)&(ut->ut_tv.tv_sec))  
> + 4, ut->ut_host);
> 	}
>
> For my personal taste the code as it was looked a little confusing,
> so i needed all the braces to understand what it was doing.
> Now it is clear at least for me.................. ;-)
>
> BTW: the coding style says to do so and i vote for it.

and then Tito suggested:

> 		if (idle < 60) {
> 			printf("00:00m    ");
> 		} else if (idle < (60 * 60)) {
> 			printf("00:%02dm    ", (int)(idle / 60));
> 		} else if (idle < (24 * 60 * 60)) {
> 			printf("%02d:%02dm    ", (int)(idle / (60 * 60)),
> 								(int)(idle % (60 * 60)) / 60);
> 		} else if (idle < (24 * 60 * 60 * 365)) {
> 			printf("%03ddays   ", (int)(idle / (24 * 60 * 60)));
> 		} else {
> 			printf("%02dyears   ", (int) (idle / (24 * 60 * 60 * 365)));
> 		}

The reason it was so unreadable to begin with was too many lines, and  
too many parentheses.  Instead of adding useless braces try removing  
some cruft.  What do you think of this:

     idle /= 60;

     if(idle < 24*60)          printf("%02d:%02dm    ", idle / 60,  
idle % 60);
     else if(idle < 24*60*365) printf("%03ddays   ",    idle / 24 /60);
     else                      printf("%02dyears   ",   idle / 24 /  
60 / 365);

It'll have to be put in another function to reduce the column count,  
but so much quicker to comprehend.
-- 
Devin Bayer



More information about the busybox mailing list