msawczyn/EFDesigner

Optimized NotifyPropertyChanged support

lizardking opened this issue · 2 comments

The current T4 files are generating code that calls OnPropertyChanged whenever the setter of a property is called (even if the value doesnt change). According to the example in How to: Implement the INotifyPropertyChanged Interface OnPropertyChanged is only called when there is a actual change of the value (also my personal preference since it avoids unecessary events).

I'm now using a modifed EFDesigner.ttinclude in my project which generates the following code for the properties:

      #region Property Name of type string
      [Required]
      protected string _Name;

      /// <summary>
      /// Required, Min length = 250
      /// </summary>
      public string Name
      {
         get {return _Name;}
         set {if (_Name != value) {string oldName=_Name; NameChanging(oldName, value);  _Name = value;OnPropertyChanged(nameof(Name)); NameChanged(oldName, value);}}
      }
      partial void NameChanging(string oldName, string newName);
      partial void NameChanged(string oldName, string newName);
      #endregion

Below is the modified method in the modified EFDesigner.ttinclude which I have included in my project. I has the improved OnPropertyChanged handling and it also excludes the ConccurencyToken from calling OnPropertyChanged.

void WritePersistentProperties(ModelClass modelClass)
{
   if (!modelClass.Attributes.Any(x => x.Persistent))
      return;

   Output("// Persistent properties");
   List<string> segments = new List<string>();
   NL();

   foreach (ModelAttribute modelAttribute in modelClass.Attributes.Where(x => x.Persistent))
   {
      string nullable = IsNullable(modelAttribute) ? "?" : "";

      Output($"#region {(modelAttribute.IsConcurrencyToken?"ConcurrencyToken":"Property")} {modelAttribute.Name} of type {modelAttribute.FQPrimitiveType}{nullable}");
      
	  segments.Clear();

      if (modelAttribute.IsIdentity)
         segments.Add("Identity");
      if (modelAttribute.Required || modelAttribute.IsIdentity)
         segments.Add("Required");
      if (modelAttribute.Indexed)
         segments.Add("Indexed");
      if (modelAttribute.MinLength > 0)
         segments.Add($"Min length = {modelAttribute.MinLength}");
      if (modelAttribute.MaxLength > 0)
         segments.Add($"Max length = {modelAttribute.MaxLength}");
      if (!string.IsNullOrEmpty(modelAttribute.InitialValue))
      {
         string quote = modelAttribute.PrimitiveType == "string" ? "\"" : modelAttribute.PrimitiveType == "char" ? "'" : "";
         segments.Add($"Default value = {quote}{FullyQualified(modelClass.ModelRoot, modelAttribute.InitialValue)}{quote}");
      }

     

      if (!modelAttribute.AutoProperty && !modelAttribute.IsConcurrencyToken)
      {
         GenerateClassAnnotations(modelAttribute, "_");
         Output($"protected {modelAttribute.FQPrimitiveType}{nullable} _{modelAttribute.Name};");

         NL();
      }

      if (!string.IsNullOrEmpty(modelAttribute.Summary) || segments.Any())
      {
         Output("/// <summary>");
         if (segments.Any())
            Output($"/// {string.Join(", ", segments)}");
         if (!string.IsNullOrEmpty(modelAttribute.Summary))
            Output("/// {0}", modelAttribute.Summary);
         Output("/// </summary>");
      }
      if (!string.IsNullOrEmpty(modelAttribute.Description))
      {
         Output("/// <remarks>");
         Output("/// {0}", modelAttribute.Description);
         Output("/// </remarks>");
      }

      string setterVisibility = modelAttribute.SetterVisibility == SetterAccessModifier.Protected ? "protected " : modelAttribute.SetterVisibility == SetterAccessModifier.Internal ? "internal " : "";

      if (modelAttribute.AutoProperty || modelAttribute.IsConcurrencyToken)
      {
         GenerateClassAnnotations(modelAttribute);
         Output($"public {modelAttribute.FQPrimitiveType}{nullable} {modelAttribute.Name} {{ get; {setterVisibility}set; }}");
      }
      else
      {

         Output($"public {modelAttribute.FQPrimitiveType}{nullable} {modelAttribute.Name}");
         Output("{");
         Output($"get {{return _{modelAttribute.Name};}}");
         Output($"{setterVisibility}set {{if (_{modelAttribute.Name} != value) {{{modelAttribute.FQPrimitiveType}{nullable} old{modelAttribute.Name}=_{modelAttribute.Name}; {modelAttribute.Name}Changing(old{modelAttribute.Name}, value);  _{modelAttribute.Name} = value;OnPropertyChanged(nameof({modelAttribute.Name})); {modelAttribute.Name}Changed(old{modelAttribute.Name}, value);}}}}");
         Output("}");
      
	     Output($"partial void {modelAttribute.Name}Changing({modelAttribute.FQPrimitiveType}{nullable} old{modelAttribute.Name}, {modelAttribute.FQPrimitiveType}{nullable} new{modelAttribute.Name});");
	     Output($"partial void {modelAttribute.Name}Changed({modelAttribute.FQPrimitiveType}{nullable} old{modelAttribute.Name}, {modelAttribute.FQPrimitiveType}{nullable} new{modelAttribute.Name});");
	  }

	   Output("#endregion");
      NL();
   }

   if (!modelClass.AllAttributes.Any(x => x.IsConcurrencyToken) && 
       (modelClass.Concurrency == ConcurrencyOverride.Optimistic || modelClass.ModelRoot.ConcurrencyDefault == Concurrency.Optimistic))
   {
      Output("/// <summary>");
      Output("/// Concurrency token");
      Output("/// </summary>");
      Output("[Timestamp]");
      Output("public Byte[] Timestamp { get; set; }");
      NL();
   }
}

Nice! Thanks!

I've given this a deep look and made the decision not to add the OnChainging and OnChanged handlers. The partials that exist so the programmer can intervene in the value that's set serve the same purpose, in the general sense, so the same codebase can dual wield for both use cases.

The nice thing, as you've discovered, is that you can change it to suit your needs. I'll be focused on the most generally useful thing (usually following the 80/20 rule as much as possible), but everyone's able and welcome to morph it to their heart's content. I did add a few fixes and removed some dead code, and those are in the v1.2.6.7 release.

I will be adding more documentation on this feature when I get a moment. I've put a placeholder in the docs and a note in the TODO list so I don't forget. You're right that this needs more explanation that I've currently given it (read: hardly any!), so I'll try to get to that as soon as time and my wife permit.

Thanks for your help in this!