patterncsharpMinor
Asynchronous Code Inspections
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
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
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
you can refactor this to
Inside the
The next problem is here
can you see it ?? A call to
At a closer look at these two
we see that at the second
Just seen the nice way how you write tenary expressions.
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.