Skip to main content
Fixed some typos
Source Link
Toby Speight
  • 88.3k
  • 14
  • 104
  • 327

###Design:

The problem I have is that you need to modify the thing you want to time (which means changing the timing).

  void functionIWantToTime()
  {
       CSDSTimer time("functionIWantToTime");

       // Stuff that takes time
  }

What would be better is a way to time a function without modifying it.

 CSDSTimer("functionIWantToTime", &functionIWantToTime);

This also allows you to time random set of stuff theatthat is not a function by using a lambda to combine them.

 CSDSTimer("Stuff", [](){
     function1();
     function2();
 )};

##Code Review

###Pass big parameters by const reference

To avoid copying objects pass by const reference.

CSDSTimer(std::string name, std::string context = "global", std::ostream &stream = std::cout)
                  ^^^^^^ Add const&  ^^^^^

###Avoid holding pointers:

std::ostream *_default_str;

You pass by reference. Why not store a reference?

###Prefer '\n' over std::endl

The difference is a flush. Flushing is usually sub optimal-optimal; the stream will flush at the optimal time. Manual flushing usually just retards performance.

###Timer Accuracy

I did not spot it. But @hoffmale suggested using a more accurate timer. I agree that is probably a good idea (you have nothing to looselose with more accuracy).

###Design:

The problem I have is that you need to modify the thing you want to time (which means changing the timing).

  void functionIWantToTime()
  {
       CSDSTimer time("functionIWantToTime");

       // Stuff that takes time
  }

What would be better is a way to time a function without modifying it.

 CSDSTimer("functionIWantToTime", &functionIWantToTime);

This also allows you to time random set of stuff theat is not a function by using a lambda to combine them.

 CSDSTimer("Stuff", [](){
     function1();
     function2();
 )};

##Code Review

###Pass big parameters by const reference

To avoid copying objects pass by const reference.

CSDSTimer(std::string name, std::string context = "global", std::ostream &stream = std::cout)
                  ^^^^^^ Add const&  ^^^^^

###Avoid holding pointers:

std::ostream *_default_str;

You pass by reference. Why not store a reference?

###Prefer '\n' over std::endl

The difference is a flush. Flushing is usually sub optimal the stream will flush at the optimal time. Manual flushing usually just retards performance.

###Timer Accuracy

I did not spot it. But @hoffmale suggested using a more accurate timer. I agree that is probably a good idea (you have nothing to loose with more accuracy).

###Design:

The problem I have is that you need to modify the thing you want to time (which means changing the timing).

  void functionIWantToTime()
  {
       CSDSTimer time("functionIWantToTime");

       // Stuff that takes time
  }

What would be better is a way to time a function without modifying it.

 CSDSTimer("functionIWantToTime", &functionIWantToTime);

This also allows you to time random set of stuff that is not a function by using a lambda to combine them.

 CSDSTimer("Stuff", [](){
     function1();
     function2();
 )};

##Code Review

###Pass big parameters by const reference

To avoid copying objects pass by const reference.

CSDSTimer(std::string name, std::string context = "global", std::ostream &stream = std::cout)
                  ^^^^^^ Add const&  ^^^^^

###Avoid holding pointers:

std::ostream *_default_str;

You pass by reference. Why not store a reference?

###Prefer '\n' over std::endl

The difference is a flush. Flushing is usually sub-optimal; the stream will flush at the optimal time. Manual flushing usually just retards performance.

###Timer Accuracy

I did not spot it. But @hoffmale suggested using a more accurate timer. I agree that is probably a good idea (you have nothing to lose with more accuracy).

Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

###Design:

The problem I have is that you need to modify the thing you want to time (which means changing the timing).

  void functionIWantToTime()
  {
       CSDSTimer time("functionIWantToTime");

       // Stuff that takes time
  }

What would be better is a way to time a function without modifying it.

 CSDSTimer("functionIWantToTime", &functionIWantToTime);

This also allows you to time random set of stuff theat is not a function by using a lambda to combine them.

 CSDSTimer("Stuff", [](){
     function1();
     function2();
 )};

##Code Review

###Pass big parameters by const reference

To avoid copying objects pass by const reference.

CSDSTimer(std::string name, std::string context = "global", std::ostream &stream = std::cout)
                  ^^^^^^ Add const&  ^^^^^

###Avoid holding pointers:

std::ostream *_default_str;

You pass by reference. Why not store a reference?

###Prefer '\n' over std::endl

The difference is a flush. Flushing is usually sub optimal the stream will flush at the optimal time. Manual flushing usually just retards performance.

###Timer Accuracy

I did not spot it. But @hoffmale suggested using a more accurate timer. I agree that is probably a good idea (you have nothing to loose with more accuracy).