This library is a cool idea. It’s something I could see myself using.
So, you’re concerned about the need to manually check/update the alarm/timer/stopwatch, and I agree that’s not great. But it also doesn’t need to be a problem. For the alarm/timer, all you need to do is wait in the destructor. This kinda sucks if you carelessly create an alarm/timer and just… forget about it… because it means there will be a “freeze” at scope end. But in that case… the coder kinda asked for it, so they can hardly complain. And it’s easy to avoid; if they get to a point where they don’t care whether the alarm/timer has fired or not, and don’t care whether the callback has run, they can always just cancel it at that point. (For the stopwatch, you don’t actually need to do anything. If you never take/check the split time or check the total time… no big dealio. 🤷🏼)
From a design perspective, the only major problem is over-complication, mostly arising from unnecessary flexibility. Two of the core guidelines of engineering design are “KISS”—or, “keep it simple, silly”—and “YAGNI”—or “you ain’t gonna need it”.
For example, the timer: the timer has, in essence, three properties:
- the delay
- the callback; and
- a flag to indicate whether it’s active.
You have made it possible to individually and separately change every single one of these properties. Is that really necessary? Is it really necessary to be able to create a timer to do foo() after 10 seconds… and then a few seconds later, change your mind and decide to do bar() instead… and then a few seconds after that, change your mind and decide it should happen 15 seconds from now… and so on. That’s a little gratuitous.
I would suggest paring it down a bit. Once a timer is created, the only “change” you should be able to make to it is to cancel it. That’s all you need. If you want a new callback behaviour, or a new delay, just create a new timer. Hell, even if you want a new callback but the same set time, just create a new timer. If you want to “reuse” a timer, just move-assign a new timer to it.
That will not only massively simplify the class, it will also solve a host of other problems. For example, if the only thing you change with an alarm is whether or not it is active, it is now trivial to make it thread-safe. If it is possible to change everything, it is practically impossible to avoid a data race, because there can be situations where you change one property, then something else interrupts concurrently before you can change the second. To prevent that, you would have to package a mutex in the alarm/timer/stopwatch… which is absurd.
The more “features” you add to a class, the greater the technical burden. You have to do more testing to cover every angle. You have to keep track of more variation in class invariants. It just makes everything harder. As a great engineer once said: “The more they overthink the plumbing, the easier it is to stop up the drain.”
The other cause of over-complication is not recognizing opportunities for reuse. For example, an alarm and a timer are exactly the same thing. Literally the only difference is one is set with a time point, and the other with a duration. Both cases could trivially be handled by a pair of constructors in a single class.
The biggest problem with the actual code is a completely ridiculous abuse of whitespace.
Just… take a look at this:
void reset ( const bool active = true)
That is literally a 100 characters wide… and for what? For this: void reset(bool active = true) (the const is also superfluous; the function is literally two lines; pretty sure there is no chance of accidentally resetting active and not noticing). 63% of the line is useless, completely devoid of meaningful information (69% of you include the unnecessary const). A short, 30 character function prologue gets stretched so wide, it won’t even fit on the screen without a horizontal scroll bar.
The logic behind such absurd horizontal spacing seems to be line up member function names… all member function names in the entire class… which means the horizontal position of all member function names is determined by the longest return type (which happens to be const time_point_type& here).
Is it really necessary to explain why this is so silly?
I mean, let’s not even consider the fact that if ever the class is modified so a return type (on any function) exceeds the current max length, you would have to edit the prologue of every single member function in the class. Even if that never happens, there is no sensible reason to lock every function name to a common horizontal position. I don’t know about you, but when I study a class, I do it one function at a time. I basically ignore the rest of the class while focusing on a single member function. I don’t care if the names don’t line up horizontally; if they do, it doesn’t help my parsing of the code at all.
And to make matters ever more ridiculous, while there is an absurd overuse of horizontal space… all the member functions are jammed together vertically with no space between. It’s hard to tell where one ends and the next begins, even without the fact that it’s hard to identify function prologues because they stretch right out of the visible window.
If you really must line up member function names… might I suggest looking into trailing return syntax. at least in that case, the longest “return type” will always be auto… a whole lot shorter than const time_point_type&.
Okay, on to the review!
#pragma once
Don’t use #pragma once. It is not standard C++, and for a very good reason.
template <typename _clock_type>
This isn’t wrong, and if you’re going to be consistent about it, that’s fine… but it is weird to name template parameters that way. The universal practice is to use UpperCamelCase for template parameters.
However, I would suggest you take advantage of concepts.
To do that, you would need a clock type concept. No problem; most of the work is already done:
template <typename T>
concept clock_type = std::chrono::is_clock_v<T>;
So now:
template <clock_type ClockType>
class alarm
{
// ...
To go even further, I would suggest templating on the callback type as well. std::function is okay for a general case, but there are many situations where it is not ideal. You may want to use std::move_only_function instead. Or you may be in a high-performance situation where neither want nor need std::function to dynamically allocate memory, and instead want to use a custom function object, like a lambda type. Even something as simple as wanting to use a function that doesn’t return void requires extra work to use if you’ve hard-coded std::function<void()> as the callback type.
And, hell, what if you don’t want a callback at all? What if you just want to know when time is up? Then your callback type could just be void.
So, like:
template <typename T>
concept clock_type = std::chrono::is_clock_v<T>;
template <
clock_type Clock,
typename Callback
>
requires std::invocable<Callback> or std::same_as<Callback, void>
class alarm
{
public:
using clock_type = Clock;
using callback_type = Callback;
constexpr explicit alarm(typename clock_type::time_point tp)
requires std::same_as<Callback, void>
: time_point_{std::move(tp)}
, active_{is_in_future(time_point_)}
{}
constexpr alarm(typename clock_type::time_point tp, callback_type cb)
requires not std::same_as<Callback, void>
: time_point_{std::move(tp)}
, callback_{std::move(cb)}
, active_{is_in_future(time_point_)}
{
// If the time is already past, do the callback immediately.
if (not active_)
std::ignore = callback_();
}
// ...
};
It would be a bit more complicated than that, because we don’t have regular void, but not by much.
I would also not bother with the ability to create “inactive” alarms. What would be the point? It’s just one more thing to test. And who would ever need to create an alarm that doesn’t actually alarm? (And even if someone does want to make an alarm object that is not set, so it can be set later, they can always do so by using move assignment: alarm = mak::time::alarm{/* new time and/or callback */};
alarm (const alarm& that) = default;
alarm ( alarm&& temp) = default;
virtual ~alarm () = default;
alarm& operator=(const alarm& that) = default;
alarm& operator=( alarm&& temp) = default;
So, first, I would say that if you’re just going to default everything, there’s really no need to spell it out like that.
Also, there is no need to give names to the parameters, which would save some keystrokes, horizontal space, and complexity.
However, I have to ask if it really makes sense for an alarm to be copyable. Let’s say you have an alarm that updates something after 10 s… what would it mean to copy that alarm? Do the update twice? Seems dangerous.
I also have to ask what is the point of making the destructor virtual, and all the class internals protected? Normally that would suggest that the intention of this class is to be a base class. But… really? In what situation would it be useful to derive from an alarm? All it does is wait a bit then call a function. What could you possibly do differently?
This is another case of YAGNI. Making a class flexible is usually a good thing… but not always; not when it comes with costs. There are costs to adding even a single virtual function to a type: now every single instance has to truck around a pointer to a vtable. And for what is everyone paying this cost? The only virtual function in your interface is the destructor. It’s not even like a derived class could change the behaviour of, say, check(). So you have a situation where you are paying a cost… and really getting nothing out of it. (You could make all the functions virtual, which would now actually make the class customizable… except now you would making every single operation a virtual call… which is even more cost.)
I would suggest not paying this useless cost at all. Don’t make the destructor virtual, don’t bother with protected; there is no reason to derive from alarm. It has one, simple behaviour: wait a bit, then call a function. There is no reason to tweak that. There is no reason to make alarm more expensive for an ability to tweak its behaviour, that no one will ever use. YAGNI.
Now, as for the move ops, they should be noexcept whenever possible… which in this case, depends on whether the callback is no-throw movable. Also, you can’t really just default them, because if you do, active_ will be true in both the moved-from and the moved-to alarms. You’ll end up with two active alarms… which, in effect, is a copy, which is probably not what you want.
And while we’re at it, you probably don’t want to default the destructor either. You probably want to do wait(), so the scheduled callback will be run, if it hasn’t been already.
So, in summary:
// Non-copyable:
alarm(alarm const&) = delete;
auto operator=(alarm const&) -> alarm& = delete;
// Move ops:
constexpr alarm(alarm&& other) noexcept(std::is_nothrow_move_constructible_v<Callback>)
: time_point_{std::move(other.time_point_)}
, callback_{std::move(other.callback_)}
, active_{std::exchange(other.active_, false)} // NOTE!
{}
constexpr auto operator=(alarm&& other) noexcept(std::is_nothrow_move_assignable_v<Callback>) -> alarm&
{
time_point_ = std::move(other.time_point_);
callback_ = std::move(other.callback_);
active_ = std::exchange(other.active_, false); // NOTE!
return *this;
}
// Destructor
~alarm() noexcept(callback_())
{
wait();
}
check() is fine, but in wait():
void wait ()
{
if (active_)
{
std::this_thread::sleep_until(time_point_);
check();
}
}
I would suggest that instead of if, you should use while. In theory, this_thread::sleep_until() should always return after the time point. But… why leave it to chance? In practice, it will probably only ever loop one or zero times. Better safe than sorry; program defensively.
[[nodiscard]]
std::future<void> async_wait ()
{
return std::async(std::launch::async, [&] { wait(); });
}
Now this is dangerous. Consider:
auto waiter = std::future<void>{};
{
auto alarm = mak::time::alarm{later, func};
waiter = alarm.async_wait();
// alarm is destroyed
}
// but waiter still holds a future that refers to alarm
If you want to allow asynchronous waiting, you might have to rethink the design.
One way to do it might be to keep a promise, and hand out shared futures. Or something like:
template <typename T>
concept clock_type = std::chrono::is_clock_v<T>;
template <
clock_type Clock,
typename Callback
>
requires std::invocable<Callback>
class alarm
{
public:
using clock_type = Clock;
using callback_type = Callback;
alarm(typename clock_type::time_point tp, callback_type cb)
{
auto promise = std::promise<std::invoke_result_t<callback_type>>{};
_future = promise.get_future();
if (clock_type::now() < _time_point)
{
auto thread = std::thread{thread_func, std::move(promise), std::move(tp), std::move(cb)};
thread.detach();
}
else
{
try
{
promise.set_value(callback());
}
catch (...)
{
promise.set_exception(std::current_exception());
}
}
}
auto get() -> decltype(auto)
{
return _future.get();
}
auto wait()
{
return _future.wait();
}
template <typename Rep, typename Period>
auto wait_for(std::chrono::duration<Rep,Period> const& d)
{
return _future.wait_for(d);
}
template <typename Clock, typename Duration>
auto wait_until(std::chrono::time_point<Clock,Duration> const& t)
{
return _future.wait_until(t);
}
private:
static auto thread_func(std::promise<std::invoke_result_t<callback_type>> promise, typename clock_type::time_point time_point, callback_type callback) -> void
{
try
{
while (clock_type::now() < _time_point)
std::this_thread::sleep_until(time_point);
promise.set_value_at_thread_exit(callback());
}
catch (...)
{
promise.set_exception_at_thread_exit(std::current_exception());
}
}
std::future<std::invoke_result_t<callback_type>> _future;
};
Of course, you could just replace the class above with a function that just returns the future.
The heart of the problem of asynchronously waiting is that you want to make sure that the alarm stays alive so long as the future exists. And there’s no practical way to do that without things getting at least a little bit expensive. Pretty much everything meant to be used concurrently—futures/promises, coroutines, etc.—all require dynamic allocation, so they can survive even outside of their scope. An asynchronous timer/alarm would probably need that too, or at least some sort of help from the OS.
So you might want to consider having a simple, high-efficiency alarm without asynchronous waiting… and a second alarm that is a bit more expensive, but allows asynchronous waiting. Indeed, that second “alarm” could be nothing more than a function that returns a void future that doesn’t turn ready until after the delay. Sorta like:
// A low-cost, high-performance, synchronous-only alarm ////////
//
// Basically: what you already have.
// Will call func at or after time_point:
auto alarm = mak::time::alarm{time_point, func};
// Will call func at or after duration:
auto alarm = mak::time::alarm{duration, func};
// If time is up and func hasn't already been called,
// calls func; otherwise returns immediately.
alarm.check();
// Wait for specified time, duration, or forever for time to be
// up and func to run:
alarm.wait();
alarm.wait_for(5s);
alarm.wait_until(later);
// If you want, you can add cancellation, so func won't be run
// even when time is up:
alarm.cancel();
// If func hasn't already been called, and it hasn't been
// cancelled, the destructor waits until time is up,
// then calls func (effectively, calls wait())"
//~alarm();
// A higher-cost, asynchronous delay ///////////////////////////
//
// Simply returns a void future that turns ready after the time
// is up.
auto future = mak::time::delay_for(5s);
auto future = mak::time::delay_until(later);
// Could be used as a synchronous alarm, to wait for whatever
// time is remaining in the delay before calling func:
future.wait(); // or future.get();
func();
// Could be used as an asynchronous alarm like so:
mak::time::delay_for(5s).and_then(func);
// The above assumes std::future will get monadic operations
// eventually. For now, you'd have to write your own:
//and_then(mak::time::delay_for(5s), func);
// or:
//chain(mak::time::delay_for(5s), func);
// or something like that.
Now, as for reset(), this can be done just as easily by move assigning a new alarm. So this function is not necessary.
// Mutators.
void set_time_point(const time_point_type& time_point)
{
time_point_ = time_point;
active_ = active_ && is_in_future(time_point_);
}
void set_callback (const callback_type& callback )
{
callback_ = callback;
}
void set_active (const bool active )
{
active_ = active;
}
// Accessors.
[[nodiscard]]
const time_point_type& time_point () const
{
return time_point_;
}
[[nodiscard]]
const callback_type& callback () const
{
return callback_;
}
[[nodiscard]]
bool active () const
{
return active_;
}
When you’re just doing get_X() and set_X() for every private data member X, that is an anti-pattern. You might as well just make the data member public.
The one exception here is the time point, because there is a little more to the logic than just setting a private data member. However, there doesn’t seem to be a need for the ability to change an alarm’s time. And if you really want to do that, you can just move assign.
Now, I mentioned that you probably don’t want to allow asynchronous waiting on a basic alarm, because that will massively increase the complexity and cost even for non-asynchronous use (though you may consider adding a second alarm class for asynchronous use). However, you can still make even the most basic alarm type safe to use across threads, for (almost!) zero cost.
The trick to making this possible is to make it so that the only variable that can change in the class is whether it’s active or not. If you allow any other data members to mutate, you invite race conditions.
If only “active” can mutate, then we can make it atomic, and get thread-safe behaviour quite easily. Well… sorta easily. As long as the callback type does not have throwing move ops.
template <clock_type Clock, std::invocable Func>
class alarm
{
std::atomic<bool> handled_;
typename Clock::time_point time_point_;
Func callback_;
public:
using clock_type = Clock;
using callback_type = Func;
// Note: constructor does *NOT* check to see if the alarm
// should have already gone off.
//
// Why? Because if the time *HAS* passed, and we call the
// callback, and it throws... we have no way of knowing
// whether it was the callback that failed, or *moving*
// the callback. So we can never know if the callback was
// run.
//
// Better to construct the alarm, then check. If the
// construction failed, we will know the callback never
// ran. If the construction succeeded, but the check failed
// we know the callback was run (but failed).
constexpr alarm(typename clock_type::time_point tp, callback_type cb)
: time_point_{std::move(tp)}
, callback_{std::move(cb)}
{}
// Note: handled_ *must* be the first data member of `other`
// that we touch. We set it to true immediately, and save
// its actual value. That way, if any other thread is
// checking the original object, it will not try to run the
// callback that has been moved-from.
//
// The only problem with this arises if the callback is not
// no-fail movable. In that case, we may mark the callback
// handled in `other`, then try to move the callback, and
// fail... but there is no way to recover the original value
// of handled in `other`. There is nothing we can do to fix
// this, so maybe you might want to ban functions with
// throwing move-ops. Or simply say that if the move does
// throw, the callback may just never be called.
//
// (Well, it is *possible* to deal with callbacks with
// throwing move-ops, but... *EXTREMELY* complicated.)
constexpr alarm(alarm&& other)
: handled_{other.exchange(true, std::memory_order::acq_rel)}
, time_point_{std::move(other.time_point_)}
, callback_{std::move(other.callback_)}
{}
constexpr ~alarm()
{
wait();
}
constexpr auto check()
{
if (_time_is_up() and not handled_.load(std::memory_order::relaxed))
{
// If we detect that handled is false, we are the
// first and only entity that could have done so.
if (handled_.exchange(true, std::memory_order::acq_rel) == false)
std::ignore = callback_();
}
}
constexpr auto wait()
{
if (not handled_.load(std::memory_order::acquire))
{
while (not _time_is_up())
std::this_thread::sleep_until(time_point_);
if (handled_.exchange(true, std::memory_order::acq_rel) == false)
std::ignore = callback_();
}
}
constexpr auto cancel() noexcept
{
handled_.store(true, std::memory_order::release);
}
// Same note about callback types with throwing move ops as
// the move constructor, though it is possible to deal with
// it here (though complicated).
constexpr auto operator=(alarm&& other) noexcept -> alarm&
{
handled_ = other.exchange(true, std::memory_order::acq_rel);
time_point_ = std::move(other.time_point_);
callback_ = std::move(other.callback_);
return *this;
}
alarm(alarm const&) = delete;
auto operator=(alarm const&) -> alarm& = delete;
};
I’m not going to bother reviewing timer because it’s basically identical to alarm. The only difference is all the gymnastics you have to do to convert time points to durations. Assuming you don’t want the ability to pause a timer, then there is no difference between alarm{clock::now() + 5s} and timer{5s} (or alarm{midnight} and timer{midnight - clock::now()}). (That is, there is no difference if the clock is steady. If the clock is not steady, then all bets are off anyway. But it would be silly to use a non-steady clock for alarms or timing, if you actually care about anything resembling accuracy.)
On to stopwatch.
So, I’m a little bit confused about how stopwatch is supposed to be used. Real-life stop watches have two buttons: start/stop, and split/reset. The latter does “split” when the stopwatch is running, and “reset” when it is not, so it is really two operations, which means a stopwatch technically has three buttons: start/stop, split, and reset. And, since start and stop are also technical distinct operations that makes four buttons. However, when I look at your interface, I see five buttons: start, stop, split, reset… and “update”. What is “update” supposed to do? (I am asking rhetorically, because I can see what it does. I am just pointing out that it doesn’t make sense from an interface perspective. So, if nothing else, it should be private.)
I would also suggest that while a real life stopwatch has those four functions, two are not really necessary. There is actually no reason a virtual stopwatch has to be able stop, or reset. In real life, it is so you can time something that may be interrupted: you start the stopwatch, then when an interruption happens, you stop it, then when the interruption is over you start it again. But this is because, in real life, it is impractical to have two (or more) stopwatches—one for before the interruption, and one for after (and another one for every other interruption that may happen). But with virtual stopwatches… why not? You can have as many as you want. There are no real practical limitations. If you want to time around interruptions, simply make another stopwatch, and then sum up the durations.
Watch how much this line of thinking simplifies things:
template <clock_type Clock>
class stopwatch
{
typename Clock::time_point _start = Clock::now();
public:
// default constructor, copy/move ops and destructor are fine
constexpr auto split() const noexcept
{
return Clock::now() - _start;
}
};
No joke, that’s it. That’s literally all you need for every conceivable use of a stopwatch.
It starts in the constructor. It never stops, but that’s fine because it’s zero cost. Since it never stops, it never needs resetting… though if you do want to “reset”, you can just do sw = stopwatch{};. And it gives the splits on demand. Everything you need.
using system_alarm = alarm<std::chrono::system_clock>;
using steady_alarm = alarm<std::chrono::steady_clock>;
using high_resolution_alarm = alarm<std::chrono::high_resolution_clock>;
using utc_alarm = alarm<std::chrono::utc_clock>;
using tai_alarm = alarm<std::chrono::tai_clock>;
using gps_alarm = alarm<std::chrono::gps_clock>;
Using some standard clocks as defaults is not a bad idea. However, trying to define an alias for every single standard clock type is a bit gratuitous. You will ultimately fall behind as new clocks are added. Even now, you’ve forgotten std::chrono::file_clock.
But there is a deeper design issue of what you are trying to say about your types and API by defining aliases like this. Doing this basically says that you think all these alarm types are valid. But… are they?
Consider a timer. If the purpose of a timer is to wait a specified time period, then the only type of clock that makes sense for a timer is a steady clock. Using a non-steady clock with a timer is arguably a mistake. If a programmer really, really wants to use a non-steady clock with a timer, then I suppose you can allow it, but you should not bless it.
So what does the above paragraph mean to an API? Well, it means that you should provide only steady clocks for the timer type. That way you send the message that only steady clocks are “correct”. Of course, if a programmer really wants to use a non-steady clock, they can… but you should not encourage that.
That would mean:
template <clock_type Clock>
class timer { /* ... */ };
using steady_timer = timer<std::chrono::steady_clock>;
// no other aliases!
But, since there is only a single steady clock, we can do even better:
template <clock_type Clock>
class basic_timer { /* ... */ };
using timer = basic_timer<std::chrono::steady_clock>;
// no other aliases!
Now a user who takes the easy path and simply uses timer gets the correct type, and the correct behaviour. But a coder who wants to risk using a non-steady clock can always do basic_timer<other_clock_type>, which is syntactically noisier, but that’s good because it is dangerous and unwise. If the coder wants to alias it to something shorter to disguise the danger of it, that’s on their head.
For alarms, a reasonable user may want an alarm that depends on system time, even though that is not steady. If a user is running a program that schedules an operation for later in the afternoon, and then the user changes the system clock… well, I mean, that’s on the user’s head. It would probably make sense for just alarm to refer to system time, but also to have aliases for UTC and TAI. GPS time is a fuzzier issue. It probably doesn’t make sense to provide aliases for file time or the high-resolution clock, or even steady_clock (because that has no real connection to real-life time points). This is something you, the designer, will have to think about.
For a stopwatch, again, the only sensible option is a steady clock, though you might also want to provide an alias to the high-resolution clock with a warning that it might give bullshit results in some situation. Or you could provide the alias if and only if std::chrono::high_resolution_clock::is_steady is true (though, I am not a fan of having APIs conditionally vary like that). Again, that is a design decision for you to make. But the key point I’m making is:
- The easiest, most natural choice should always be the best choice. For timers and stopwatches, the best choice is to use a steady clock. So that’s what
timer and stopwatch should do.
- Other choices that are okay, but not the best, should be possible, but require a bit more from the coder to get at. Using a non-steady clock with a timer or stopwatch is okay, so long as the risks are understood. Requiring the coder to spell out
basic_timer<std::chrono::high_resolution_clock> forces them to think a little bit harder about whether that’s really what they want to do (it almost never is).
- Choices that are just plain objectively wrong should be banned completely. For example, using anything but a clock type as the template parameter. This is why I recommended using a concept.
That is the essence of a good API design.
And finally:
}
When a stray brace appears in a source code file hundreds (or, sometimes thousands) of lines away from its sibling, it’s a good idea to comment it so the connection is clear. For example:
namespace mak::time {
// ... ~400 lines of code ...
} // namespace mak::time
That’s all for the review!
In summary:
Avoid over-complication. Figure out the essence of a thing, and only add the functionally required to capture that essence.
All timer needs to do is run its function when time has expired, and maybe be cancelled (that is, be told not to run the function when time has expired); it does not need to be able to be enabled/disabled repeatedly, have its set time changed repeated, or have its function changed repeatedly. All that unnecessary stuff adds is complexity, and it often makes extra capabilities (like being thread-safe) difficult or impossible.
Even when functionality is “free” to add (it doesn’t require extra data members or efficiency costs)… resist the temptation. Every thing a class can do must be tested, so no functionality is really free. KISS, and YAGNI.
Less horiztonal whitespace, more vertical whitespace.
Lining up everything horizontally makes little sense… and it makes absolutely zero sense when you are lining up stuff that isn’t even visible on a single screen. Unless stuff is directly next to each other, vertically—like with a list of type aliases— horizontal alignment doesn’t matter. (For example, horizontally aligning function prologues when entire function bodies separate them? Absolutely ridiculous.)
Asynchronous support is cool, but expensive, because it usually requires dynamic allocation to allow stuff to exist across non-overlapping scopes.
It’s probably best to keep simple types simple, and, if desired, have asynchronous types available only as an option for those willing to pay the cost.
The design of an API signals to users how a library or class is supposed to be used. The natural, correct, and intended use should be the easiest thing to do. Indeed, doing anything else should often be hard. Yes, allowing flexibility is a good thing, but whenever something would be dangerous or unwise, it should be harder to do, if not impossible.
That’s all! Have a good one!
main()that shos intended usage? Showing those would help reviewers understand the code more quickly. \$\endgroup\$