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

Evaluating Parser State

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

Problem

The "parser state" of a module in rubberduck can be one of several values:

//note: ordering of the members is important
public enum ParserState
{
    /// 
    /// Parse was requested but hasn't started yet.
    /// 
    Pending,
    /// 
    /// Project references are being loaded into parser state.
    /// 
    LoadingReference,
    /// 
    /// Code from modified modules is being parsed.
    /// 
    Parsing,
    /// 
    /// Parse tree is waiting to be walked for identifier resolution.
    /// 
    Parsed,
    /// 
    /// Resolving identifier references.
    /// 
    Resolving,
    /// 
    /// Parser state is in sync with the actual code in the VBE.
    /// 
    Ready,
    /// 
    /// Parsing could not be completed for one or more modules.
    /// 
    Error,
    /// 
    /// Parsing completed, but identifier references could not be resolved for one or more modules.
    /// 
    ResolverError,
}


Now, Rubberduck uses each modules' state, and determines what the "overall" state is - and then displays that value in a status bar.

The rules are:

  • If all modules have the same state, overall state is that value.



  • If any module is in an error state, overall state is that error value.



  • Overall state can only be "ready" when all modules are "ready".



  • Overall state can only be "parsed" when all modules are "parsed".



  • Otherwise return the state of the most advanced non-ready module.



The code started simple, got complicated, then too simple, and now looks like this - basically it went back to "too complicated":

```
private static readonly ParserState[] States = Enum.GetValues(typeof(ParserState)).Cast().ToArray();
private ParserState EvaluateParserState()
{
var moduleStates = _moduleStates.Values.ToList();
var state = States.SingleOrDefault(value => moduleStates.All(ps => ps == value));

if (state != default(ParserState))
{
// if all modules are in the same state, we have our result.
Debug.WriteLine("ParserState evalua

Solution

-
If I understand the rules correctly, the last rule "Otherwise return the state of the most advanced non-ready module." isn't met by your method. Having states {ParserState.LoadingReference, ParserState.Parsed, ParserState.Parsing, ParserState.Ready} I would expect the returned state equals to ParserState.Parsed but your method results in ParserState.Parsing.

-
The rules 3 and 4 are superflous because they are satisfied by rule 1.

-
Because any error state takes precedence over every other state you should chewck this first. If all states are of the same error state it will result in the same result, but checking first the error states will be faster.

-
this

if (result == ParserState.Ready && moduleStates.Any(item => item != ParserState.Ready))
{
    result = moduleStates.Except(new[] {ParserState.Ready}).Max();
}


looks strange. result equals to the Min() of the modulStates, so let us assume it would equal to ParserState.Ready then it is garanteed that the second condition will result in true because you already checked if all states are the same. Because of the the check of the same values the second condition of Debug.Assert(result != ParserState.Ready || moduleStates.All(item => item == ParserState.Ready)); is superflous as well.

-
You should use the same variables name for the same type when using lambdas. One time you have ps => ps == value and the other times you use ms => ms == ParserState.Error.

-
The method relies on a class variable which I would change. Add a parameter to that method declaration so it can stand on its own.

Implementing the mentioned points will lead to

private ParserState EvaluateParserState(IList moduleStates)
{
    // error state takes precedence over every other state
    if (moduleStates.Any(ms => ms == ParserState.Error))
    {
        Debug.WriteLine("ParserState evaluates to '{0}' (thread {1})", ParserState.Error,
        Thread.CurrentThread.ManagedThreadId);
        return ParserState.Error;
    }
    if (moduleStates.Any(ms => ms == ParserState.ResolverError))
    {
        Debug.WriteLine("ParserState evaluates to '{0}' (thread {1})", ParserState.ResolverError,
        Thread.CurrentThread.ManagedThreadId);
        return ParserState.ResolverError;
    }

    var state = States.SingleOrDefault(value => moduleStates.All(ms => ms == value));

    if (state != default(ParserState))
    {
        // if all modules are in the same state, we have our result.
        Debug.WriteLine("ParserState evaluates to '{0}' (thread {1})", state, Thread.CurrentThread.ManagedThreadId);
        return state;
    }

    var result = moduleStates.Except(new[] { ParserState.Ready }).Max();

    Debug.Assert(result != ParserState.Ready);

    Debug.WriteLine("ParserState evaluates to '{0}' (thread {1})", result,
    Thread.CurrentThread.ManagedThreadId);
    return result;
}

Code Snippets

if (result == ParserState.Ready && moduleStates.Any(item => item != ParserState.Ready))
{
    result = moduleStates.Except(new[] {ParserState.Ready}).Max();
}
private ParserState EvaluateParserState(IList<ParserState> moduleStates)
{
    // error state takes precedence over every other state
    if (moduleStates.Any(ms => ms == ParserState.Error))
    {
        Debug.WriteLine("ParserState evaluates to '{0}' (thread {1})", ParserState.Error,
        Thread.CurrentThread.ManagedThreadId);
        return ParserState.Error;
    }
    if (moduleStates.Any(ms => ms == ParserState.ResolverError))
    {
        Debug.WriteLine("ParserState evaluates to '{0}' (thread {1})", ParserState.ResolverError,
        Thread.CurrentThread.ManagedThreadId);
        return ParserState.ResolverError;
    }

    var state = States.SingleOrDefault(value => moduleStates.All(ms => ms == value));

    if (state != default(ParserState))
    {
        // if all modules are in the same state, we have our result.
        Debug.WriteLine("ParserState evaluates to '{0}' (thread {1})", state, Thread.CurrentThread.ManagedThreadId);
        return state;
    }

    var result = moduleStates.Except(new[] { ParserState.Ready }).Max();

    Debug.Assert(result != ParserState.Ready);

    Debug.WriteLine("ParserState evaluates to '{0}' (thread {1})", result,
    Thread.CurrentThread.ManagedThreadId);
    return result;
}

Context

StackExchange Code Review Q#126714, answer score: 5

Revisions (0)

No revisions yet.