-
Notifications
You must be signed in to change notification settings - Fork 193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
4847 logger singleton 3.1 #5119
Changes from 1 commit
2d0a76c
659d01c
3984b2b
aad08ee
69c474b
c162c04
442cd38
96d111a
bca7f1b
bf372b5
a13db45
db83481
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,18 +8,14 @@ | |
|
||
#include "../UtilitiesAPI.hpp" | ||
|
||
#include "Singleton.hpp" | ||
#include "Exception.hpp" | ||
#include "Compare.hpp" | ||
#include "Compare.hpp" // NOTE that this this is not needed for this header but so many compilation errors if I omit it | ||
// since other files transitively use Compare | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. An unfortunate discovery. Cleaning up ours headers would be good, but that's not what I'm trying to achieve here. |
||
#include "LogMessage.hpp" | ||
#include "LogSink.hpp" | ||
|
||
#include <boost/shared_ptr.hpp> | ||
#include "Exception.hpp" | ||
|
||
#include <sstream> | ||
#include <set> | ||
#include <map> | ||
#include <memory> | ||
#include <shared_mutex> | ||
|
||
/// defines method logChannel() to get a logger for a class | ||
#define REGISTER_LOGGER(__logChannel__) \ | ||
|
@@ -51,21 +47,24 @@ | |
namespace openstudio { | ||
|
||
class OSWorkflow; | ||
class LogSink; | ||
namespace detail { | ||
class Logger_Impl; | ||
class LogSink_Impl; | ||
} // namespace detail | ||
|
||
/// convenience function for SWIG, prefer macros in C++ | ||
UTILITIES_API void logFree(LogLevel level, const std::string& channel, const std::string& message); | ||
|
||
class Logger; | ||
|
||
/** Primary logging class. | ||
*/ | ||
class UTILITIES_API LoggerImpl | ||
class UTILITIES_API Logger | ||
{ | ||
friend class Logger; | ||
|
||
public: | ||
/// destructor, cleans up, writes xml file footers, etc | ||
~LoggerImpl(); | ||
static Logger& instance(); | ||
|
||
Logger(const Logger& other) = delete; | ||
Logger(Logger&& other) = delete; | ||
Logger& operator=(const Logger&) = delete; | ||
Logger& operator=(Logger&&) = delete; | ||
|
||
/// get logger for standard out | ||
LogSink standardOutLogger() const; | ||
|
@@ -94,40 +93,10 @@ class UTILITIES_API LoggerImpl | |
void addTimeStampToLogger(); | ||
|
||
private: | ||
/// private constructor | ||
LoggerImpl(); | ||
|
||
mutable std::shared_mutex m_mutex; | ||
|
||
/// standard out logger | ||
LogSink m_standardOutLogger; | ||
|
||
/// standard err logger | ||
LogSink m_standardErrLogger; | ||
Logger(); | ||
~Logger() = default; | ||
|
||
/// map of std::string to logger | ||
using LoggerMapType = std::map<std::string, LoggerType, openstudio::IstringCompare>; | ||
LoggerMapType m_loggerMap; | ||
|
||
/// current sinks, kept here so don't destruct when LogSink wrapper goes out of scope | ||
using SinkSetType = std::set<boost::shared_ptr<LogSinkBackend>>; | ||
SinkSetType m_sinks; | ||
}; | ||
|
||
class UTILITIES_API Logger | ||
{ | ||
public: | ||
Logger() = delete; | ||
|
||
static LoggerImpl& instance() { | ||
if (!obj) { | ||
obj = std::shared_ptr<LoggerImpl>(new LoggerImpl()); | ||
} | ||
return *obj; | ||
} | ||
|
||
private: | ||
static std::shared_ptr<LoggerImpl> obj; | ||
std::shared_ptr<detail::Logger_Impl> m_impl; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given that the Logger is a Meyers singleton, I'm not sure whether the Logger_Impl class is really needed at all. But it feels familiar at least, and can allow us to shorten the public includes. The dowside is that there's an extra method call for each (the forward to impl) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, I don't think we really need an Impl at all. |
||
}; | ||
|
||
} // namespace openstudio | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Private constructor, and public ::instance()