On 24/07/19 06:32PM, Markus Elfring wrote: > … > >> +++ 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? No I think I prefer the current version of the code. But my judgement does not matter much here, lets wait what the maintainers have to say about this.
Attachment:
signature.asc
Description: PGP signature
