Only on monolith code bases without any signs of modularity.
Only on monolith code bases without any signs of modularity.
In other words, don't make it look complicated before it actually is.
That was kind of the gist of my comment.
foo.log(xyz);
bar.log(42);
vs log_event(foo, xyz);
log_event(bar, 42);
Provided that both approaches do the same, which is more clear? It is the second one, because it clearly suggests that there is no polymorphic switching based on foo/bar.Wading through non-trivial projects, pervasiveness of the first approach makes me nervous, because I'm told at every occasion, "don't even try to understand what's going on in the system globally. We .log(), shouldn't that be enough for you to know?".
While the second approach is calming to me, "You see, we have this simple logging system, where everything ends up being written to. It's not that complex after all, and if you want to know what's going on you can just jump directly to the log_event()" function.
Yes, by asserting that foo and bar are of the same type, and asserting that the .log() method is not virtual, I can reach the same conclusion. But that's the point, it requires work that you can't practically do if each line of code contains a method call. There is no mincing words, you're using the wrong syntax...
First foo or bar can be function pointers.
Or maybe log_event is a macro whose behavior depends on the environment. And I have used some crazylogging ones back in the 2000's when coding C, that would even dump the current state of the process alongside its parameters.
Or then again, log_event() plugs into a configurable logging system and one cannot predict its behavior just from the call site, even when using the same types.
Finally if we move away from C semantics, maybe log_event is overloaded or is doing multiple dispatch, thus the two calls, depending on the argument types, are not going to land on the same implementation.
Then different uses of the same identifier still resolve to the same implementation, and make the IDE jump to the same location, etc.
(I acknowledge that everything can be something else. And in theory you can redefine macros to have uses of the same identifier resolve to different implementations. Etc, etc. I'm sure you know about the IOCCC. But here we talk about good programming practice. It's a good practice not to suggest something complicated while the reality is simple. There's no argument to win here.)
> Finally if we move away from C semantics, maybe log_event is overloaded or is doing multiple dispatch, thus the two calls, depending on the argument types, are not going to land on the same implementation.
Which is why I do not use C++, or at least avoid most of its features. For me, it's all about clarity and reducing number of moving parts. It's not a contest in making complicated things that pretend to be simple. And it's not a contest in making simple things that look complicated. And it's not a contest in reducing the number of characters in individual identifiers at all costs.
so, question : suppose you have your log_event, and now your bosses comes and tells you that the client needs to have two additional different logging mechanisms - say, to journald and to some websocket log server. The system can log to either of your three logging mechanisms at any point through changing a configuration somewhere in a GUI but you also need some core classes to always be logged through journald no matter the state of the rest of the system.
How do you update your design to reflect that ?
What is needed in the face of changing requirements is actually this: lean structure with few dependencies that can be adapted.
That wasn't hard.
Also you've made your logging library project-specific by making a log_core_event function. Thanks but I'll keep using spdlog in my projects, which do not need newcomers to learn about your log_core_event, what is a core module and what is not, just to be able to write the three functions they are called for in that project !
That all has nothing to do with virtual methods.
> thread initialization if you have multiple threads doing logging on startup, right ? :-) )
There is no technical difference between "global" and "local" variables. It's merely a syntactical difference.
If you run into concurrency problems with global variables, you'll run into problems just as well with objects. Unless you make multiple objects. In which case, you could also have done thread-local storage, no?
In the end, it's much clearer to me to use global variables (TLS or not) since this way I can actually see the data relations. If there is just one instance of a thing, why would I pass it around as objects? It's not going to help understandability.
> Also you've made your logging library project-specific by making a log_core_event function.
That doesn't make sense. Can't I just add another function without breaking everything? Note that I probably wouldn't even add this function to "the logging library" but to the core modules...
> what is a core module and what is not
If you prefer having everything switched behind your back, resulting in unreadable and unintelligible source code... you do you.
More technical notes...
If you don't like the presence of two or more functions for logging, then why not have an explicit context argument? You call log_event(MODULE_FOO, foo, xyz) instead, and the log_event function implements all the plumbing logic in a central place, easily understood (Most logging approaches do that, I think. They have "facility" and "severity" arguments). This approach seems much more preferable to me compared to an implementation that you can't see locally, and that can't be influenced locally. What would you do if we go for the OOP approach with a virtual method, and now call into a different module that would need a different logger?
If having a dynamically switched implementation was really what we want to do, there is no problem with doing that. My post was just about using method syntax for static things, which I think harms readability.
well, that cuts short to the discussion. There are plenty of places & coding standards where those are outright forbidden.
> If you prefer having everything switched behind your back, resulting in unreadable and unintelligible source code... you do you.
we have very different notions of readable. For me the less stuff I have to read and the more readable it is - I just want my code to be the pure domain problem and could not give a rat's ass about how logging or networking or whatever is implemented if those aren't my domain.
> Can't I just add another function without breaking everything?
that does not break the code but that breaks Joe intern's mental model and expectations of anyone who will think that log_* functions are from the same library
> then why not have an explicit context argument?
and here we are, reinventing OOP by hand in C. There is literally no difference between log_event(context, ...) and context.log_event(...) - but you get better tooling :
* completion -for the first I'd most certainly end up typing the full name if there are a lot of l-started functions while for the second I'd type c<shortcut>.l<shortcut><enter> and only see the few relevant functions in the autocompletion list) etc... in the second.
* namespacing - if I want to see all the logging functions available I can just click on "context" and my IDE will happily show me all the available methods in a panel and nothing else. With C functions, well, I have to scroll in a list of 150000 functions. And also wonder (& have to explain) why log_event() is in <util/logger.h> and log_event_core is in <core/core_logger.h>.
* split implementations - if at some point you want to port your software to, say, an arduino without network capabilities (talking literally forom past experience here), the monolithic log_event function now becomes an #ifdef HAS_NETWORK / #ifdef HAS_JOURNALD / ... mumbo-jumbo. While with multiple logger subtypes it's just a matter of changing your build system to not compile the unavailable implementations, without even needing to change the code (or change a list of types in a header if you use static polymorphism instead).
> What would you do if we go for the OOP approach with a virtual method, and now call into a different module that would need a different logger?
dependency injection ? modules can request either for a default logger which will be sourced from the gui configuration, or whatever specific logger they actually need due to functional requirements.
Besides, I still don't see the readability problem with virtuals. Again in my IDE if I want to see "what happens" I press F2 on context.log_event and it displays me the list of all the possible implementations if it isn't able to determine that one is used statically at that point. And I much prefer to read three five-lines email_logger::log, journald_logger::log, websocket_logger::log functions than a pile of if/else's.
But when I had a glance at the project you linked on github, that was basically the first thing I found there...
A suggestion that I have there would be to use the linker instead (make different source files based on implemntation / existance of an implementation). This way you can eliminate most of the #ifdefs.
Regarding dependency injection, I don't see a practical difference to just using global variables / global functions, apart from the fact that by convention DI gives you back objects with virtual implementations, which I don't like.
I won't go into the rest of the things you said because we pretty much discussed it all.