Skip to content
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

Code Cluthering, SRP, KISS, 10/100, ... Violation? #109

Open
db-007 opened this issue Nov 10, 2017 · 0 comments
Open

Code Cluthering, SRP, KISS, 10/100, ... Violation? #109

db-007 opened this issue Nov 10, 2017 · 0 comments

Comments

@db-007
Copy link

db-007 commented Nov 10, 2017

I like MetroLog idea but I have 2 problems.

Take a look at the code I borrowed from your documentation below

    protected override void OnNavigatedTo(NavigationEventArgs e)
    {
        base.OnNavigatedTo(e);
        
        // flat strings...
        if (this.Log.IsInfoEnabled)
            this.Log.Info("I've been navigated to.");

        // formatting...
        if (this.Log.IsDebugEnabled)
            this.Log.Debug("I can also format {0}.", "strings");

        // errors...
    		try
    		{
    			this.DoMagic();
    		}
    		catch(Exception ex)
    		{
                if (this.Log.IsWarnEnabled)
    				this.Log.Warn("You can also pass in exceptions.", ex);
    		}
    }

The code above pollutes otherwise nice and clean code with lots of logging calls. It gets even worse if you add to it exception handling, instrumentation calls (i.e. to measure the execution time of each method by using StopWatch, etc) and the method that should have 1 or 2 lines of code became a tangled complex spaghetti ball.

This violates KISS, SRP, 10/100 and bunch of other principles. The method above should really look like this:

    protected override void OnNavigatedTo(NavigationEventArgs e)
    {
        base.OnNavigatedTo(e);
        
    	this.DoMagic();
    }

It would be nice if MetroLog could be used as AOP (Aspect Oriented Programming) library, have you ever thought about that? That way, the method above could be used something like this:

    [LogAspect]
    protected override void OnNavigatedTo(NavigationEventArgs e)
    {
        base.OnNavigatedTo(e);
        
    	this.DoMagic();
    }

Where all the logging statements could be moved into a new class LogAspect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant