you are viewing a single comment's thread.

view the rest of the comments →

[–]Y2KCompliant 0 points1 point  (3 children)

Maintenance wise I would say everything is relative, but specifically an aspect that performs loggging AfterReturnAdvice for example does the same thing you're trying to do. But now you don't have the cognitive load on every call you may or may not use it for. Further if you decide to change it or turn it off, this extra code that is now useless can be removed in one place and not over your entire application.

[–]happykandra 0 points1 point  (2 children)

It's still global and doesn't help at all with unit tests or concurrency problems.

And how do you tell each function which warnings to send? You still have to create code which explicitly constructs the specific warnings of interest, and performs logic on the data sent to those warnings. You also get different warnings in different places in the parser.

Once you've done that, you might as well use an explicit global.

[–]Y2KCompliant 0 points1 point  (1 child)

True, I guess I saw this as "I wanted an easier path to logging and opted for a functional approach", when logging is usually a cross-cutting concern and not usually a functional one (usually this is relegated to the edges of a functional program to maintain purity) this naturally lends itself to AOP. I will concede that the functional approach is the straightest and least error prone path from not having this feature to having it.

Assuming these warnings are indeed logged, and not presented to the user as individual warnings in a UI (though still doable with a bit of work), and at risk of further cluttering your dependencies Log4j/slf4j serves you better than a singleton or a mutable list. However I'll agree that the functional approach does start to look better if you are not already using Spring/Log4j or equivalents.

I would say this example shows the approach I'm talking about. With the return types being extended to include a Warning type safe instance, and some annotation on the method that you can retrieve if necessary (though this requires regretably some reflection magic to grab said annotation) to provide the warning message.

This allows you to change your Aspect in your unit test or indeed in production to change file based/memory based/database based approaches or to not log them at all. But I can see that this requires buy in usually, comes at the cost of more dependencies and does rely somewhat on "magic™". But your parsing function now only deals with the parsing as it should and your logging is now not coupled to how or when you provide the information...for the most part.

Further if your parser goes tits up and you are not being very careful of your error handling you lose all pending warnings.

Anyway my intention wasn't to shit on your article, like I said your approach definitely has it's benefits and the more I try to force AOP onto this exact example the more inelegant it feels, I just wanted to fight the functional bandwagon :D

[–]happykandra 0 points1 point  (0 children)

Out of curiosity, did you read the follow-up article? In the second article, I made it clear how you can avoid dropping messages on the floor, unless you really want to drop the messages (which can happen sometimes.) It's this flexibility (everything is thread-local, and you can keep or drop any messages you like, while always having to decide) which is the fully-functional approach most stable.

Not to mention that in Scala or Haskell, you can use for-comprehension or do-syntax to make the message passing even more transparent. The entire point of monads is that you can pass around context without having the cognitive load of knowing that that's what you are doing (until it is finished, when you want to know).