Re: [PATCH] perf trace: Avoid duplicate code in fprintf_duration()

[Markus Elfring]

…
>> +++ b/tools/perf/builtin-trace.c
>> @@ -1258,12 +1258,16 @@ static size_t fprintf_duration(unsigned long t, bool calculated, FILE *fp)
>>
>>  	if (!calculated)
>>  		printed += fprintf(fp, "         ");
>> -	else if (duration >= 1.0)
>> -		printed += color_fprintf(fp, PERF_COLOR_RED, "%6.3f ms", duration);
>> -	else if (duration >= 0.01)
>> -		printed += color_fprintf(fp, PERF_COLOR_YELLOW, "%6.3f ms", duration);
>>  	else
>> -		printed += color_fprintf(fp, PERF_COLOR_NORMAL, "%6.3f ms", duration);
>> +		printed += color_fprintf(fp,
>> +					 (duration >= 1.0
>> +					 ? PERF_COLOR_RED
>> +					 : (duration >= 0.01
>> +					   ? PERF_COLOR_YELLOW
>> +					   : PERF_COLOR_NORMAL)),
>> +					 "%6.3f ms",
>> +					 duration);
>
> Why is this a desirable change?

I find it helpful to specify the affected function call only once
in such an if branch.


>                                 Folding the if-statements into the
> ternary operator makes the code quite unreadable compared to what it was
> like before and doesn't give any obvious improvement.

Do you prefer to store the result of the colour determination into another
local variable so that it can be passed as a separate parameter?

Regards,
Markus