This repository has been archived by the owner on Feb 13, 2024. It is now read-only.
[WIP] Introduce a polymorphic Error class #4517
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📒 Description
There is a lot of text so please don't get scared. I just want to know your opinion, especially on the topic if this is overengineered or not from your point of view.
This PR will introduce an error-focused suite of classes to handle errors in a more manageable manner.
All of the new
Error
API is implemented in thesrc/util/error.h
file and it's rather short.For now, the API design is as follows:
ErrorBase
is an abstract class implementing a basic API to work with errors. It provides the following methods:std::string Class()
returns the name of the error class (likeDatabaseError
orNoError
for example)int Type()
is the type of the error - this can (doesn't have to) be provided by the class implementing this API. It is intended to be used for different types of errors, like with the database, I have currently implemented types for example for:PROGRAMMING_ERROR
(when a null argument is passed),SESSION_ERROR
(when a SQL query fails),INCONSISTENT_DATA
(when we're trying to update a nonexistent value for example)<something>Error
class implementation and will be different for a futureTimeEntryError
bool IsError()
to determine if it's actually an error (onlyNoError
will return false in here)std::string LogMessage()
will return a message to be printed to the log (and probably sent to Bugsnag as well)std::string UserMessage()
will return a message to be displayed to the user (which will very often be an empty string)Error
is a wrapper class that is never used on its own. It acts as a return value to all methods that were returningerror
before. I needed to implement it like this because abstract (polymorphic) classes in C++ have to be passed around as pointers if we want to benefit from the polymorphism (assigning aDatabaseError
value to aErrorBase
variable "slices" it, removing theDatabaseError
part of the value).template<class T> Error(T &&e)
is the most important method (and a constructor) of this class because it allows implicit conversion of allErrorBase
-based classes to (for example)Error<DataBaseError>
which can be passed around safely (and since it contains a smart pointer, the memory will be freed automatically when it goes out of scope)ErrorBase
instance and working with it in an easy manner.Now, that I've described the internals (which seem really convoluted but don't worry, they allow for something that I consider very nice further down the road), I can put an example of the usage below:
I can now declare a class called
DatabaseError
and that will contain an enum with a few error types:Now, all this is just an example, the
UserMessage
handling doesn't really have to be anenum
with amap
, we can of course implement a class that will handle this as a string that will get constructed somewhere else - OR we don't have to show anything to the user at all.With this class implemented, we can then rewrite the
Database
class and change theerror
usages toDatabaseError
:BEFORE:
AFTER:
Notice how the error messages are now classified according to where they are coming from.
🕶️ Types of changes
🤯 List of changes
👫 Relationships
Closes #4515
🔎 Review hints
There will have to be some discussion on which errors need to be reported to which channels (shown to the user, written to log, sent to bugsnag).