I am making a very simple stopwatch functionality as below.

New will start the timer.

Pause will suspend the watch.

Start will restart again.

Session will return the duration.

#include<chrono>
#include<vector>

class Timer 
 using cppTimer = std::chrono::high_resolution_clock;
 using seconds = std::chrono::seconds;
 std::vector< int64_t > mvSession;
 cppTimer::time_point mtpNow;
 cppTimer::time_point mtPause;
 seconds mtRestart;
 bool mbIsPause;
public:

 enum Units 
 sec,
 min,
 hrs,
 end
 ;

 Timer() noexcept: mbIsPause(false), mtRestart ,mvSession(3) 
 

 inline void Now() noexcept 
 mtpNow = cppTimer::now();
 mbIsPause = false;
 mtRestart = seconds(0);
 

 inline void Pause() noexcept 
 if(!mbIsPause) 
 mtPause = cppTimer::now();
 mbIsPause = true;
 

 

 inline void Start() noexcept 
 if(mbIsPause) 
 mtRestart += std::chrono::duration_cast<std::chrono::seconds>(cppTimer::now() - mtPause);
 mbIsPause = false;
 

 


 std::vector< int64_t > SessionTime() 

 auto now = cppTimer::now();
 auto delay = std::chrono::duration_cast<std::chrono::seconds>(now - mtpNow);

 if(mbIsPause) 
 mtRestart += std::chrono::duration_cast<std::chrono::seconds>(cppTimer::now() - mtPause);
 

 auto tp = std::chrono::duration_cast<std::chrono::seconds>(delay - mtRestart); //+ mtRestart ;// SecondsTp(cppTimer::now()) - SecondsTp(mtpNow) + SecondsTp(mtRestart);

 mvSession[sec] = tp.count()%60;
 mvSession[min] = std::chrono::duration_cast<std::chrono::minutes>(tp).count()%60;
 mvSession[hrs] = std::chrono::duration_cast<std::chrono::hours>(tp).count();

 return mvSession;
 

;

Don't use member variables for temporary storage

Your class Timer has a member variable mvSession, which is unnecessary. It is only used in SessionTime(), where it is filled in and returned. No other functions use it. Instead, just declare std::vector<int64_t> mvSession(3) inside SessionTime().

By making it a member variable, you introduced two problems: first, your class now uses more memory than necessary. Second, the function SessionTime() is now no longer reentrant; if two threads would call SessionTime() simultaneously, they would both write to the same mvSession variable, potentially corrupting it.

Use std::chrono::steady_clock for timers

The problem with std::chrono::high_resolution_clock() is that it is not guaranteed what kind of clock it is. It can represent wall clock time, which can jump ahead or forwards because of summer and winter time, leap seconds, the computer suspending and resuming. This is not something you want for a timer where you are interested in a simple duration. For this, you want to use std::chrono::steady_clock, which is guaranteed to be a monotonically increasing clock. Also, your function SessionTime() returns the time in a resolution of seconds, so you don't need a high resolution clock anyway.

Don't use Hungarian notation

There might be some merits to Hungarian notation, but it really isn't necessary to use it for C++, since the compiler will do type checking for you. Moreover, it's easy to use the wrong prefix, and it's hard to come up with a reasonable prefix when you can have complex types.

You are already making mistakes in your code. For example, mtpNow and mtPause are both of type cppTimer::time_point. So the prefix should have been the same. And mtRestart has a different type than mtPause, so their prefixes should have been different. I recommend that you just avoid using Hungarian notation altoghether.

Be consistent with using

You are declaring using seconds = std::chrono::seconds, and use seconds in a few places, but you also use std::chrono::seconds in a lot of places. Furthermore, you also use std::chrono::minutes and std::chrono::hours, but have not declared an alias for them. In this case, I suggest you don't declare using seconds at all.

I would keep using cppTimer though, since it basically selects which clock to use. That makes it easier to change the clock later by just changing one line of code. I would write using clock = ... though, to be consistent with the C++ library itself.

Don't cast to seconds too early

Instead of seconds mtRestart, use cppTimer::duration mtRestart. This will keep the accuracy of the duration to the same as the clock itself. Only cast durations to seconds or other time intervals until the last moment possible. The same goes for the calculation of delay in SessionTime(), just write:

auto delay = now - mtpNow;

Use nouns for variable names, verbs for function names

A variable holds (the value of) a thing, so its name should naturally be a noun. Functions do stuff, so there names should generally be verbs. The function Now() should actually be named Start(). Your function Start() should probably be named Continue(). The function SessionTime() calculates how long the timer has been running for, so probably should be named GetDuration().

Conversely, the variables mtPause and mtRestart should be renamed to nouns as well. They are a bit confusing. Sure, you set mtPause in the Pause() function, but it doesn't describe what the value actually means. The same goes for mtRestart. I would instead write:

clock::time_point StartTime;
clock::time_point PauseTime;
clock::duration PausedDuration;
bool IsPaused;

Now you can rewrite the function Now() to:

void Start() 
 StartTime = clock::now();
 IsPaused = false;
 PausedDuration = ;

Remove mtRestart

You are using two variables to handle the timer being paused, mtPause and mtRestart. However, you only need one. In the Pause() function, you indeed just record when this function is called. However, when restarting the timer, instead of adding the duration of being paused to mtRestart, just add that duration to mtpNow instead:

void Start() 
 if(mbIsPause) 
 mtpNow += cppTimer::now() - mtPause;
 mbIsPause = false;
 

This also simplifies SessionTime():

std::vector<int64_t> SessionTime() 
 auto end = mbIsPause ? mtPause : cppTimer::now();
 auto tp = std::chrono::duration_cast<std::chrono::seconds>(end - mtpNow);
 ...

Also, since mtPause is only ever 0 when you didn't pause, you can use that to signal whether the timer is paused instead of having bool mbIsPause.

Another option is @user673679's suggestion of storing only the start time and the accumulated elapsed time so far. You would then use the start time being equal to as a signal that the timer has not been started.

Just return a std::chrono::duration

When you want the elapsed time, I would avoid having the Timer class be responsible for converting the duration to a vector of integers representing hours, minutes and seconds. It reduces the accuracy of your timer. Instead, I would just return a std::chrono::duration, and have the caller decide whether it wants to convert that to something. It also is much more efficient than having to construct a std::vector<int64_t>.

Try to make it behave reasonable in all situations

One issue with your code is that it only gives reasonable results for SessionTime() if you have called Now() at least once. You didn't initialize mtpNow, and even if it was value-initialized to zero, then SessionTime() will return the time that has passed since the epoch.

If you want the Timer to behave like it was started at construction time, then initialize mtpNow to cppTimer::now(). If you want it to behave like it was paused, then ensure both mtpNow and mtPause are initialized to the same value (I suggest just using ), and that mbIsPause is true.

Make it work like a real stopwatch

As already suggested by others, it helps to think of a timer as a stopwatch. A real stopwatch starts in a stopped state, showing an elapsed time of zero. Then, you can start and stop the timer mechanism with buttons. Usually, there is a separate button to reset the stopwatch to its initial state. By making the class act like something a lot of people are already familiar with, the code is easier to understand for others.

Reworked code

Here is an example of what the code would look like with my suggestions applied, as well as @user673679's way of storing the elapsed time between previous start and stops of the clock:

#include <chrono>

class Timer 
 using clock = std::chrono::steady_clock;
 clock::time_point StartTime = ;
 clock::duration ElapsedTime = ;

public:
 bool IsRunning() const 
 return StartTime != clock::time_point;
 

 void Start() 
 if(!IsRunning()) 
 StartTime = clock::now();
 
 

 void Stop() 
 if(IsRunning()) 
 ElapsedTime += clock::now() - StartTime;
 StartTime = ;
 
 

 void Reset() 
 StartTime = ;
 ElapsedTime = ;
 

 clock::duration GetElapsed() 
 auto result = ElapsedTime;
 if(IsRunning()) 
 result += clock::now() - StartTime;
 
 return result;
 
;
```

• It's fine to prefix member variables with an m, but it's probably best to avoid type prefixes (e.g. mtpNow). It makes code harder to read (you have to know what every abbreviation means), it's a pain to maintain (e.g. mtPause should probably be mtpPause to be consistent). Modern tools eliminate the need for this too (mousing over a variable in Visual Studio will tell me the exact type, not an approximation).




• Functions defined inline in the class don't need to be declared inline.



• 
  

  The naming is very confusing:

  
  
  

  • Now() differs from the standard library (now() returns the current time, and is arguably still a terrible name). Functions names should be commands or questions. Perhaps Restart() or Reset() would be better.

  
  

  • Start() is also not ideal. One might expect the function to do what Now() does. I'd suggest calling it Unpause(), which makes the purpose clearer.

  
  

  • mtRestart is an odd name for the time spent paused.

  
  



• We don't really need the Now() / Restart() function, since we can just assign a new timer to the old one to do the same thing (e.g. Timer timer; ...; timer = Timer(); // restarted!).




• There's no reason to keep a vector in the class (we're copying it every time anyway, so we might as well just create a new vector each time).




• We don't need a vector, since it always has 3 values, it would be neater to return a simple struct, which would allow us to give each value a name. Or...




• The real solution is to just return an appropriate chrono:: type, and let the user worry about formatting / converting it.




• I'd suggest naming the class Stopwatch, since that's more specific to the pausable timing functionality we need. We can actually implement this functionality based on a simpler, non-pausable Timer class.

So overall I'd suggest something more like this (not tested properly):

#include <chrono>

template<class ClockT = std::chrono::high_resolution_clock>
class Timer

 ClockT::time_point m_start;

public:

 Timer():
 m_start(ClockT::now())
 

 

 ClockT::duration GetElapsedTime() const
 
 return ClockT::now() - m_start;
 
;

template<class ClockT = std::chrono::high_resolution_clock>
class Stopwatch

 bool m_paused;
 Timer<ClockT> m_timer;
 ClockT::duration m_timeAccumulated;

public:

 explicit Stopwatch(bool paused = false):
 m_paused(paused),
 m_timer(),
 m_timeAccumulated(0)
 

 

 void Pause()
 
 m_timeAccumulated += m_timer.GetElapsedTime();
 m_paused = true;
 

 void Unpause()
 
 if (m_paused)
 
 m_timer = Timer<ClockT>();
 m_paused = false;
 
 

 ClockT::duration GetElapsedTime() const
 
 if (m_paused)
 return m_timeAccumulated;

 return m_timeAccumulated + m_timer.GetElapsedTime();
 
;

#include <iostream>

int main()

 using seconds_t = std::chrono::duration<float>;

 Stopwatch s;

 for (int i = 0; i != 50; ++i) std::cout << "."; 
 std::cout << "n";

 std::cout << std::chrono::duration_cast<seconds_t>(s.GetElapsedTime()).count() << std::endl;

 s = Stopwatch(true);
 std::cout << std::chrono::duration_cast<seconds_t>(s.GetElapsedTime()).count() << std::endl;

 s.Unpause();

 for (int i = 0; i != 50; ++i) std::cout << "."; 
 std::cout << "n";

 s.Pause();
 std::cout << std::chrono::duration_cast<seconds_t>(s.GetElapsedTime()).count() << std::endl;

 for (int i = 0; i != 50; ++i) std::cout << "."; 
 std::cout << "n";

 std::cout << std::chrono::duration_cast<seconds_t>(s.GetElapsedTime()).count() << std::endl;