[GitHub] [logging-log4cxx] ams-tschoening commented on a change in pull request #24: Implement mapfilter

[GitBox <gi...@apache.org>]

ams-tschoening commented on a change in pull request #24:
URL: https://github.com/apache/logging-log4cxx/pull/24#discussion_r434354711



##########
File path: src/main/include/log4cxx/filter/mapfilter.h
##########
@@ -19,26 +19,87 @@
 
 #include <log4cxx/spi/filter.h>
 
+#if defined(_MSC_VER)
+	#pragma warning ( push )
+	#pragma warning ( disable: 4251 )
+#endif
+
 namespace log4cxx
 {
 namespace filter
 {
+/**
+ * A Filter that operates on a Map.
+ */
 
+ using keyvallist = std::map<LogString, LogString>;
 
 class LOG4CXX_EXPORT MapFilter: public log4cxx::spi::Filter
 {
+	private:
+		bool acceptOnMatch{true};

Review comment:
       C++11-features like this one need to be changed most likely to support legacy-compilers, simply because I'm still using one myself. :-/
   
   http://docwiki.embarcadero.com/RADStudio/Rio/en/C%2B%2B11_Features_in_the_Classic_Compiler

##########
File path: src/main/include/log4cxx/filter/mapfilter.h
##########
@@ -19,26 +19,87 @@
 
 #include <log4cxx/spi/filter.h>
 
+#if defined(_MSC_VER)
+	#pragma warning ( push )
+	#pragma warning ( disable: 4251 )
+#endif
+
 namespace log4cxx
 {
 namespace filter
 {
+/**
+ * A Filter that operates on a Map.
+ */
 
+ using keyvallist = std::map<LogString, LogString>;

Review comment:
       That type reads somewhat badly, how about using `KeyValList` or `key_val_list` instead? I prefer the former, like most other custom types are named and that scheme is used by your variables as well.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org