Opened 14 years ago
Last modified 5 years ago
#408 new enhancement
[PATCH] Use exception strings instead of LOGERROR in (de)serializer
Reported by: | Philip Taylor | Owned by: | |
---|---|---|---|
Priority: | If Time Permits | Milestone: | Backlog |
Component: | Simulation | Keywords: | patch |
Cc: | Patch: |
Description
In some cases the serialization code uses LOGERROR then throws an exception. Now that exceptions can contain message strings, the code should simply throw the exception and not log it explicitly, to simplify the code a little.
Attachments (2)
Change History (16)
comment:1 by , 14 years ago
Milestone: | Unclassified |
---|
comment:2 by , 14 years ago
Milestone: | → Backlog |
---|
comment:3 by , 11 years ago
I believe this is mostly fixed, with a handful of cases in BinzarySerializer.cpp
remaining.
comment:4 by , 11 years ago
The remainding errors in BinarySerializer.cpp have dynamic messages which cannot be thrown so easy with the current error class.
Either modify the error class so it creates a local copy of the string or use a static string buffer.
Any reason PSERROR inherits from std::exception and not from std::runtime_error? (That one would have its own string copy and what is also implemented.)
comment:5 by , 11 years ago
Cc: | added |
---|
I don't know why PSERROR was implemented that way, jan or Philip would know, maybe it was personal preference, but I do know it has annoyed me for a long time that it only accepts a temp C-string.
comment:6 by , 11 years ago
Sure, why not change to to runtime_error. I don't know why it isn't already.
comment:7 by , 11 years ago
We now could add two interfaces to allow simple composing:
- Adding a constructor with variable argument list (like printf).
throw SomeException("Error: %i", errno);
- Adding a stringsstream to allow C++ like formatting.
throw SomeException() << "Error: " << errno;
I favor the latter as the compile knows best what type the variables (e.g. errno) has.
(Or none of them, the exception just takes one simple string.)
by , 11 years ago
Attachment: | use-pserror-stream.patch added |
---|
comment:8 by , 11 years ago
Keywords: | patch review added |
---|---|
Summary: | Use exception strings instead of LOGERROR in (de)serializer → [PATCH] Use exception strings instead of LOGERROR in (de)serializer |
Needs some testing with different compilers as well.
Added a small test case. (Any additional test cases?)
by , 11 years ago
Attachment: | pserror-stream.patch added |
---|
comment:9 by , 11 years ago
Milestone: | Backlog → Alpha 15 |
---|
comment:10 by , 11 years ago
Not sure I care for the stringstream approach, it looks very strange compared to how we handle errors and strings in most of the engine :/ The printf style for better or worse is more conventional in the engine.
comment:11 by , 11 years ago
Cc: | removed |
---|---|
Type: | task → enhancement |
comment:12 by , 11 years ago
Owner: | changed from | to
---|
comment:13 by , 11 years ago
Keywords: | review removed |
---|---|
Milestone: | Alpha 15 → Backlog |
Owner: | removed |
comment:14 by , 5 years ago
Component: | UI & Simulation → Simulation |
---|
Move tickets to Simulation
as UI & Simulation
got some sub components.
Milestone Unclassified deleted