CODEWITHSUNDEEP
c++performance
Luca Davanzo
Luca Davanzo

Reputation: 21528

Performance vs Readability with optional parameter

Suppose that I have to call same method for each member of a vector contained in a class.
If element-i of the vector has a certain value, I have to modify it.
I've max 10 element in this vector.

What is the best way to do?
- Call a method with an optional parameter?
- Or call a method with an if-else statment inside?

class A {   
  public:    
   A();   
   void doSomething(const string& s) {   
      if(s=="foo") {  
        std::cout << s;   
      } else {   
        std::cout << s << ".dat"   
      }   
    }    
    void doSomethingDef(const string& s, const string& v = "")  {   
      std::cout << s << v;   
    }      
  private:   
    std::vector<std::string> list_;      
}  

int main() {   
  A a;  
}  


/* Constructor with method 1 */   
A::A() {    
 std::vector<std::string>::iterator i = list_.begin();     
 for(; i != list_.end(); ++i) {    
   this->doSomething(*i);    
 }     
}   

/* Constructor with method 2 */   
A::A() {    
   this->doSomethingDef(list[0]);   
   this->doSomethingDef(list[1]);   
   this->doSomethingDef(list[2]);  
   this->doSomethingDef(list[3]);    
   this->doSomethingDef(list[4], ".dat");  
   this->doSomethingDef(list[5], ".dat");  
   this->doSomethingDef(list[6]);  
   this->doSomethingDef(list[7]);  
   this->doSomethingDef(list[8]);  
   this->doSomethingDef(list[9]);  
}

Upvotes: 0

Views: 107

Answers (3)

Jerry Coffin
Jerry Coffin

Reputation: 490663

If you decided to use something similar to your second method (not that I'm advising it, just considering how to do it if you were to), I'd strongly consider using overloading instead of a default parameter:

void doSomethingDef(const string& s)  {   
  std::cout << s;   
}

void doSomethingDef(const string &s, const string &v) {
   std::cout << s << v;
}

With a default parameter, you're still creating a temporary string and passing it as the second parameter, then making all the iostreams calls to insert that string into the stream even though it has no effect. By using overloading instead, you avoid all the processing necessary to create, pass, and insert the second string (except when you really have a second string to display, of course).

Mixing that with I/O (and only writing 10 strings anyway) probably covers up most of the benefit from this, so it'll probably be hard to even measure, not to mention notice in real life. On the other hand, if you were generating a lot of strings and (for example) depositing the output in memory instead of writing it to output, the difference could start to become substantial.

Upvotes: 0

YoungJohn
YoungJohn

Reputation: 956

In C++11 your constructor with method 1 could be written as:

A::A()
{
    for(const auto & listElement : list_)
    {
        this->doSomething(listElement);
    }
}

Performance and readability and a bit more generic to boot. Or you could do it with a std::for_each and a lambda function which may trigger a compiler with optimizations turned on to unroll that loop for you.

A::A()
{
    for_each(begin(list_), end(list_),[this](const std::string & listElement)
    {
        doSomething(listElement);
    });
}

Upvotes: 1

Sebastian Redl
Sebastian Redl

Reputation: 72112

If you have a max of 10 elements, I'm going to claim that performance is completely irrelevant anyway.

Furthermore, until you have proven that performance really matters, method 1 is so much more readable that you should prefer it anyway.

Finally, because of the way cout operations work, I would guess that in this little example, method 2 is actually slower, because it's faster to do a branch and a comparison with a fixed string than doing an additional call to a formatted output function.

Upvotes: 3

Related Questions