trjhtr

We have a client that handles three different sort of licenses. These are up for renewal every year and upon renewal they need to check between 15-20 rules to see that everything is okay. They do this manually today but everything they check is within their business system so they have all the data. If any deviations show up these should be presented in a browser.

To solve this we decided to use flags and let one bit represent one deviation. The client has confirmed they can not see that they need more than 20 deviations and they have been doing this for a lot of years.

The back end is C# and front end is a react application with TypeScript.

We use TypeLITE to generate TypeScript definitions from our C# classes.

Given that bitwise operators use 32 bits in JavaScript we are limited to that in the back end as well.

1 << 32 //1 in javascript

Bitwise operators

See this code and tests and please see if anything can be done better.

RuleBook:

public class DeviationRuleBook

 private readonly List<ValidationRule> rules;

 public DeviationRuleBook()
 
 rules = new List<ValidationRule>();
 

 public int Count => rules.Count;

 public void AddRule(ValidationRule rule)
 
 if (rule == null)
 
 throw new ArgumentNullException(nameof(rule));
 

 rules.Add(rule);
 

 public ValidationResult RuleCheck(TPRenewalCycle tpCase)
 
 var result = new ValidationResult()
 
 Status = ValidationStatus.Passed
 ;

 foreach (ValidationRule rule in rules)
 rule.ErrorType;
 

 return result;
 

Validate cycle:

public static class HandleDeviation

 public static ValidationResult ValidateRenewalCycle(TPRenewalCycle tpRenewalCycle)
 
 DeviationRuleBook ruleBook = null;

 if (tpRenewalCycle?.TPCase == null)
 
 return new ValidationResult()
 
 Status = ValidationStatus.Failed,
 Errors = ValidationErrorType.NoCaseForRenewalCycle
 ;
 

 if (tpRenewalCycle.TPCase.PropertyType == PropertyTypes.License1)
 
 ruleBook = DeviationRuleBookFactory.License1RuleBook();
 
 else if (tpRenewalCycle.TPCase.PropertyType == PropertyTypes.License2)
 
 ruleBook = DeviationRuleBookFactory.License2RuleBook();
 
 else if (tpRenewalCycle.TPCase.PropertyType == PropertyTypes.License3)
 
 ruleBook = DeviationRuleBookFactory.License3RuleBook();
 

 var validationResult = ruleBook.RuleCheck(tpRenewalCycle);

 return validationResult;
 

Different licenses requires different rules:

public static class DeviationRuleBookFactory

 public static DeviationRuleBook License1RuleBook()
 
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new RegistrationNumberExistsRule());
 rulebook.AddRule(new ApplicationNumberMissingRule());
 rulebook.AddRule(new FeeCalculationRule());
 return rulebook;
 

 public static DeviationRuleBook License2RuleBook()
 
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new RegistrationNumberExistsRule());
 rulebook.AddRule(new ApplicationNumberMissingRule());
 rulebook.AddRule(new FeeCalculationRule());

 return rulebook;
 

 public static DeviationRuleBook License3RuleBook()
 
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new RegistrationNumberExistsRule());
 rulebook.AddRule(new FeeCalculationRule());
 return rulebook;
 

Abstract class for rules:

/// <summary>
/// Validation rule ensuring that an entity conforms to some kind of rule.
/// </summary>
public abstract class ValidationRule

 /// <summary>
 /// Get information about the validation result.
 /// </summary>
 public bool IsValid get; protected set; 

 /// <summary>
 /// Perform validation.
 /// </summary>
 public abstract void Validate(TPRenewalCycle tpCase);

 public abstract ValidationErrorType ErrorType get; 

Example of rule:

public class RegistrationNumberExistsRule : ValidationRule

 public override void Validate(TPRenewalCycle cycle)
 
 if (cycle == null)
 return;

 var exist = cycle.TPCase?.RegistrationNo != null && cycle.TPCase?.RegistrationDate != null;

 IsValid = exist;
 

 public override ValidationErrorType ErrorType => ValidationErrorType.RegistrationNumberExists;

Result:

[TsClass(Module = "Application")]
public class ValidationResult

 public ValidationStatus Status get; set; 
 public ValidationErrorType Errors get; set; 

Enums:

[TsEnum(Module = "Application")]
public enum ValidationStatus

 Passed = 0,
 Failed = 1


//Can only use 32-bit due to JavaScript limitation
[Flags]
[TsEnum(Module = "Application")]
public enum ValidationErrorType : Int32

 None = 0,
 NoCaseForRenewalCycle = 1 << 0, // 1
 RegistrationNumberExists = 1 << 1, // 2
 ApplicationNumberMissing = 1 << 2, // 4
 FeeCalculationNoPrice = 1 << 3, // 8
 DesignatedCountriesExists = 1 << 4, // 16

Test rule example:

[TestFixture]
public class RegistrationNumberExistsRuleTests

 [Test]
 public void RegistrationNumberExist()
 

 var tpRenewalCycle = new TPRenewalCycle();

 tpRenewalCycle.TPCase = new TPCase();

 tpRenewalCycle.TPCase.RegistrationNo = "Test";

 tpRenewalCycle.TPCase.RegistrationDate = DateTime.Now;

 var rule = new RegistrationNumberExistsRule();

 rule.Validate(tpRenewalCycle);

 Assert.IsTrue(rule.IsValid);
 

 [Test]
 public void RegistrationNumberDoesNotExist()
 

 var tpRenewalCycle = new TPRenewalCycle();

 tpRenewalCycle.TPCase = new TPCase();

 tpRenewalCycle.TPCase.RegistrationDate = DateTime.Now;

 var rule = new RegistrationNumberExistsRule();

 rule.Validate(tpRenewalCycle);

 Assert.IsFalse(rule.IsValid);
 

Test rule book example:

[TestFixture]
public class DeviationRuleBookTests


 [Test]
 public void PassedRuleCheckShouldBeTrueWhenNoRulesInBook()
 
 var cycle = new TPRenewalCycle();
 var rulebook = new DeviationRuleBook();

 var result = rulebook.RuleCheck(cycle);

 Assert.IsTrue(result.Status == ValidationStatus.Passed);
 

 [Test]
 public void PassedRuleCheckShouldBeFalseWhenAnyRuleInBookAreInvalid()
 
 var cycle = new TPRenewalCycle();
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new ValidRule());
 rulebook.AddRule(new InvalidRule());

 var result = rulebook.RuleCheck(cycle);

 Assert.IsTrue(result.Status == ValidationStatus.Failed);
 

 [Test]
 public void ShouldIncludeAllFailedRulesInError()
 
 var cycle = new TPRenewalCycle();
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new ValidRule());
 rulebook.AddRule(new RegistrationNumberExistsRule());
 rulebook.AddRule(new ApplicationNumberMissingRule());

 var result = rulebook.RuleCheck(cycle);

 Assert.IsTrue(result.Errors.HasFlag(ValidationErrorType.RegistrationNumberExists));
 Assert.IsTrue(result.Errors.HasFlag(ValidationErrorType.ApplicationNumberMissing));
 

 [Test]
 public void ShouldOnlyIncludeErrorTypeForFailedRules()
 
 var cycle = new TPRenewalCycle();
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new ValidRule());
 rulebook.AddRule(new RegistrationNumberExistsRule());
 rulebook.AddRule(new ApplicationNumberMissingRule());

 var result = rulebook.RuleCheck(cycle);

 var errorCount = 0;
 foreach (ValidationErrorType errorType in Enum.GetValues(typeof(ValidationErrorType)))
 
 if (errorType != 0 && result.Errors.HasFlag(errorType))
 
 errorCount++;
 
 

 Assert.IsTrue(errorCount == 2);
 

 class ValidRule : ValidationRule
 
 public override void Validate(TPRenewalCycle cycle)
 
 IsValid = true;
 

 public override ValidationErrorType ErrorType => ValidationErrorType.None;
 

 class InvalidRule : ValidationRule
 
 public override void Validate(TPRenewalCycle cycle)
 
 IsValid = false;
 

 public override ValidationErrorType ErrorType => ValidationErrorType.None;
 

TypeLite generated files:

interface ValidationResult 
 Errors: Application.ValidationErrorType;
 Status: Application.ValidationStatus;


export const enum ValidationErrorType 
 None = 0,
 NoCaseForRenewalCycle = 1,
 RegistrationNumberExists = 2,
 ApplicationNumberMissing = 4,
 FeeCalculationNoPrice = 8

export const enum ValidationStatus 
 Passed = 0,
 Failed = 1

Can check for a flag like this in Front End:

let cycles = ValidationErrorType.NoCaseForRenewalCycle | ValidationErrorType.ApplicationNumberMissing; 

if (cycles & ValidationErrorType.NoCaseForRenewalCycle) 
 console.log("NoCaseForRenewalCycle rule hit");

For the most part this looks pretty good to me, good work!

I only really have a few comments.

1. Don't use Assert.IsTrue to check if two things are equal. Prefer Assert.AreEqual - this will result in better error messages when the test fails.



2. 
   

   Don't save something to a variable only to assign it to another variable, it's just noise.

   
   
   

   var exist = cycle.TPCase?.RegistrationNo != null && cycle.TPCase?.RegistrationDate != null;
   
   IsValid = exist;
   

   
   
   

   Is better written as:

   
   
   

   IsValid = cycle.TPCase?.RegistrationNo != null && cycle.TPCase?.RegistrationDate != null;
   

   
   



3. I might be missing something, but ValidationResult.Status isn't necessary - at least, it isn't necessary to handle setting it. It could be implemented by just checking ValidationResult.Errors




4. HandleDeviation.ValidateRenewalCycle has a problem. In the code you have provided you have 3 license types. If you need to add another license type, you have to add another if statement. Every time a new license is added the cyclomatic complexity increases. It would be better to store a Dictionary<PropertyTypes, Action> of the factory methods.

I would still question your decision to use binary encoding for storing the deviations. It has several downsides besides the extensibility problem:

• Leads to more complex code - bitwise operations rarely make things easier to understand.


• Harder to debug - seeing number 5 sent over network, which flags does it represent?

Text is widely considered a much better protocol format. If you decide to go binary, you better have reasons for doing so:

• Do you have high bandwidth demands? Sending and receiving gigabytes of license data every day, so you'll need to squeeze down every extra byte to save precious bandwidth.


• Do you have high CPU demands? Processing millions of licenses every second, you need to squeeze the most out of CPU by using fast binary operations.


• Is your data binary in nature? Images, Video, Cryptography, Machine code...

I doubt that these apply in your case. Perhaps you have some other valid reason.

If you don't have a strong need to use binary... don't. The future maintainers will thank you.

A big thanks to @Gerrit0 and @ReneSaarsoo for their input. I skipped using flags and ValidationResult.Status. Code based on comments:

Validate cycle:

public static class HandleDeviation

 public static IEnumerable<DeviationType> ValidateRenewalCycle(TPRenewalCycle tpRenewalCycle)
 
 if (tpRenewalCycle?.TPCase == null)
 
 return new List<DeviationType>() DeviationType.NoCaseForRenewalCycle;
 

 DeviationRuleBook ruleBook = null;

 var ruleBookDictionary = new Dictionary<string, DeviationRuleBook>();
 ruleBookDictionary.Add(PropertyTypes.License1, DeviationRuleBookFactory.License1RuleBook());
 ruleBookDictionary.Add(PropertyTypes.License2, DeviationRuleBookFactory.License2RuleBook());
 ruleBookDictionary.Add(PropertyTypes.License3, DeviationRuleBookFactory.License3RuleBook());

 ruleBookDictionary.TryGetValue(tpRenewalCycle.TPCase.PropertyType, out ruleBook);

 var validationResult = ruleBook.RuleCheck(tpRenewalCycle);

 return validationResult;
 

RuleBook:

public class DeviationRuleBook

 private readonly List<ValidationRule> rules;

 public DeviationRuleBook()
 
 rules = new List<ValidationRule>();
 

 public int Count => rules.Count;

 public void AddRule(ValidationRule rule)
 
 if (rule == null)
 
 throw new ArgumentNullException(nameof(rule));
 

 rules.Add(rule);
 

 public IEnumerable<DeviationType> RuleCheck(TPRenewalCycle tpCase)
 
 var result = new List<DeviationType>();

 foreach (ValidationRule rule in rules)
 
 rule.Validate(tpCase);
 if (rule.IsValid)
 
 continue;
 

 result.Add(rule.DeviationType);
 

 return result;
 

Test rule book example:

[TestFixture]
public class DeviationRuleBookTests


 [Test]
 public void PassedRuleCheckShouldBeTrueWhenNoRulesInBook()
 
 var cycle = new TPRenewalCycle();
 var rulebook = new DeviationRuleBook();

 var result = rulebook.RuleCheck(cycle);

 Assert.IsTrue(!result.Any());
 

 [Test]
 public void PassedRuleCheckShouldBeFalseWhenAnyRuleInBookAreInvalid()
 
 var cycle = new TPRenewalCycle();
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new ValidRule());
 rulebook.AddRule(new InvalidRule());

 var result = rulebook.RuleCheck(cycle);

 Assert.IsTrue(result.Any());
 

 [Test]
 public void ShouldIncludeAllFailedRulesInError()
 
 var cycle = new TPRenewalCycle();
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new ValidRule());
 rulebook.AddRule(new RegistrationNumberExistsRule());
 rulebook.AddRule(new ApplicationNumberMissingRule());

 var result = rulebook.RuleCheck(cycle);

 Assert.IsTrue(result.Any(x => x == DeviationType.RegistrationNumberExists));
 Assert.IsTrue(result.Any(x => x == DeviationType.ApplicationNumberMissing));
 

 [Test]
 public void ShouldOnlyIncludeErrorTypeForFailedRules()
 
 var cycle = new TPRenewalCycle();
 var rulebook = new DeviationRuleBook();
 rulebook.AddRule(new ValidRule());
 rulebook.AddRule(new RegistrationNumberExistsRule());
 rulebook.AddRule(new ApplicationNumberMissingRule());

 var result = rulebook.RuleCheck(cycle);

 var errorCount = 0;
 foreach (DeviationType errorType in Enum.GetValues(typeof(DeviationType)))
 
 if (errorType != 0 && result.Any(x => x == errorType))
 
 errorCount++;
 
 

 Assert.AreEqual(errorCount, 2);
 

 class ValidRule : ValidationRule
 
 public override void Validate(TPRenewalCycle cycle)
 
 IsValid = true;
 

 public override DeviationType DeviationType => DeviationType.None;
 

 class InvalidRule : ValidationRule
 
 public override void Validate(TPRenewalCycle cycle)
 
 IsValid = false;
 

 public override DeviationType DeviationType => DeviationType.None;
 

Enum:

[TsEnum(Module = "Application")]
public enum DeviationType : Int32

 None = 0,
 NoCaseForRenewalCycle = 1,
 RegistrationNumberExists = 2,
 ApplicationNumberMissing = 3,
 FeeCalculationNoPrice = 4,
 DesignatedCountriesExists = 5,