HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Asynchronous Code Inspections

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
asynchronouscodeinspections

Problem

Rubberduck code inspections can take quite a while to complete, so I decided to run them on a background thread instead of locking up the IDE. The only drawback is... the IDE isn't locked-up while inspections are running, so the user can end up with inspection results that go against stale parse results - this could mean stale inspection results (say you add Option Explicit to a module that didn't have it), or stale token positioning (say you add or remove lines of code; every inspection result refering to a token located below that will be misplaced when navigating).

But the UX is worth it.. I think.

The status label indicates when parsing is happening...

...and when inspections are running:

All results are populated at once in the grid:

And there's a distinct status when no issues are found:

Here's the presenter code:

```
namespace Rubberduck.UI.CodeInspections
{
public class CodeInspectionsDockablePresenter : DockablePresenterBase
{
private CodeInspectionsWindow Control { get { return UserControl as CodeInspectionsWindow; } }

private IEnumerable _parseResults;
private IList _results;
private readonly IInspector _inspector;

public CodeInspectionsDockablePresenter(IInspector inspector, VBE vbe, AddIn addin, CodeInspectionsWindow window)
:base(vbe, addin, window)
{
_inspector = inspector;
_inspector.IssuesFound += OnIssuesFound;
_inspector.Reset += OnReset;
_inspector.Parsing += OnParsing;
_inspector.ParseCompleted += OnParseCompleted;

Control.RefreshCodeInspections += OnRefreshCodeInspections;
Control.NavigateCodeIssue += OnNavigateCodeIssue;
Control.QuickFix += OnQuickFix;
Control.CopyResults += OnCopyResultsToClipboard;
}

// indicates that the _parseResults are no longer in sync with the UI
private bool _needsResync;

private void OnParseComplete

Solution

Just a few quick shoots..

To reduce the small code duplication here

private void OnParseCompleted(object sender, ParseCompletedEventArgs e)
{
    ToggleParsingStatus(false);
    if (sender == this)
    {
        _needsResync = false;
        _parseResults = e.ParseResults;
        Task.Run(() => RefreshAsync());
    }
    else
    {
        _parseResults = e.ParseResults;
        _needsResync = true;
    }
}


you can refactor this to

private void OnParseCompleted(object sender, ParseCompletedEventArgs e)
{
    ToggleParsingStatus(false);
    _parseResults = e.ParseResults;
    _needsResync = sender != this
    if (!needsResync)
    {
        Task.Run(() => RefreshAsync());
    }
}


Inside the private async Task RefreshAsync() method you have some more "problems". First you should reduce the horizontal spacing by using a guard clause on if (VBE == null) and return early.

The next problem is here

var parseResults = _parseResults.SingleOrDefault(p => p.Project == VBE.ActiveVBProject);


can you see it ?? A call to SingleOrDefault() will based on the name return either a single item or a default item. A single item != plural ! So rename the local variable to parseResult.

At a closer look at these two if statements

if (_parseResults == null || _needsResync)
    {
        _inspector.Parse(VBE, this);
        return;
    }

    var parseResults = _parseResults.SingleOrDefault(p => p.Project == VBE.ActiveVBProject);
    if (parseResults == null || _needsResync)
    {
        _inspector.Parse(VBE, this);
        return;
    }


we see that at the second if condition we can omit the || _needsResync because this won't ever be true.

Just seen the nice way how you write tenary expressions.

Code Snippets

private void OnParseCompleted(object sender, ParseCompletedEventArgs e)
{
    ToggleParsingStatus(false);
    if (sender == this)
    {
        _needsResync = false;
        _parseResults = e.ParseResults;
        Task.Run(() => RefreshAsync());
    }
    else
    {
        _parseResults = e.ParseResults;
        _needsResync = true;
    }
}
private void OnParseCompleted(object sender, ParseCompletedEventArgs e)
{
    ToggleParsingStatus(false);
    _parseResults = e.ParseResults;
    _needsResync = sender != this
    if (!needsResync)
    {
        Task.Run(() => RefreshAsync());
    }
}
var parseResults = _parseResults.SingleOrDefault(p => p.Project == VBE.ActiveVBProject);
if (_parseResults == null || _needsResync)
    {
        _inspector.Parse(VBE, this);
        return;
    }

    var parseResults = _parseResults.SingleOrDefault(p => p.Project == VBE.ActiveVBProject);
    if (parseResults == null || _needsResync)
    {
        _inspector.Parse(VBE, this);
        return;
    }

Context

StackExchange Code Review Q#90798, answer score: 6

Revisions (0)

No revisions yet.