Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

State machine refactoring #60

Merged
merged 5 commits into from
Nov 6, 2023

Conversation

Whitehouse112
Copy link
Contributor

@Whitehouse112 Whitehouse112 commented Nov 3, 2023

  • Removed ValueTable Signal property (obsolete)
  • Removed IsSigned Signal property (obsolete)
  • Added FSM to parse ValueTable string into dictionary
  • Updated Demo application to correctly display ValueTableMap dictionary
  • Updated some tests
  • Added new tests
  • Updated README.md

Copy link
Collaborator

@Adhara3 Adhara3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO StringToDictionaryStateMachine would be cleaner with a static bool ParseString() method and a ctor that takes the IDictionary as parameter

public static bool ParseString(string text, out IReadOnlyDictionary<int, string> dictionary)
{
    //State = States.FindingIndex; this goes in the constructor
    dictionary = null;
    var internalDictionary = new Dictionary<int, string>();
    var parser = new StringToDictionaryStateMachine(internalDictionary);
    if(parser.ProcessEvent(text, Events.Start))
    {
          dictionary = internalDictionary;
          return true;
    }
    return false;
}

As another note, I would try to avoid the .Substring calls when not necessary, just keep carrying the current index with you and keep the original text as is without cropping it.

Final note: each method/state is setting a state and then calls a generic method, it could instead call the method directly, that would allow specific signatures an may simplify the state

@Adhara3 Adhara3 merged commit ea5d9e3 into EFeru:main Nov 6, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants