Re: [PATCH 04/11] trace-cmd: Extract parse_record_options() from trace_record()

[Steven Rostedt]

On Tue, 28 Nov 2017 20:57:13 +0200
Vladislav Valtchev <vladislav.valtchev@xxxxxxxxx> wrote:

> On Tue, 2017-11-28 at 13:30 -0500, Steven Rostedt wrote:
> > On Tue, 28 Nov 2017 20:17:46 +0200
> > Vladislav Valtchev <vladislav.valtchev@xxxxxxxxx> wrote:
> >   
> > > Since that code was profile-specific, I put it in trace_profile(),
> > > without 'if' statements, clearly.  
> > 
> > This is all about balancing. You were able to remove one if statement,
> > but required two function calls by all others.
> >   
> 
> I totally agree that is all about balancing.
> I wrote exactly the same thing as part of my previous e-mail (before reading this message).
> 
> Are you concerned by the cost the of function calls or by the "verbosity" in calling them?

No the cost is negligible. More about the complexity. Adding "paired"
calls can sometimes be more confusing than if logic.

> 
> Otherwise, I believe that having those function calls that way just makes the code simpler
> for new contributors. Functions, among everything else, are also "labels" for pieces of code.
> Having two different labels there (init context and parse options) makes sense to me.
> But, again, I understand that is a trade-off based on my subjective value system and
> way of thinking, of course.
> 
> Do you have a strong opinion?

Yes ;-)

I'll reply why in the other email.

-- Steve