The registration system - part 2



  • As promised, here's the start of the fun parts of code. Well, first let me start by sharing that there was a happy ending. The 17 project, 98k line monstrosity, without a unit test in sight, was transformed to a 3 project 16k line set of components. Oh, and that 16k line count included a suite of tests to validate every part of the system. And, everyone on the team can just as easily support the system. I started the rewrite on my own and on my own time but it was a team effort at the end to tidy everything up, get it QAed and into production.

    Anyway, on to the code...

    As I said, the problem at hand was simple, read one or several records from the database, display, and potentially write to the database. The read side had several distinct patterns:

    • DB -> Strongly typed dataset -> hierarchical object model -> flattened object model -> UI
    • DB -> Strongly typed dataset -> dataset -> hierarchical object model -> flattened object model -> UI
    • DB -> Strongly typed dataset -> dataset -> UI
    • DB ->  dataset -> UI 

    The added beauty of this was that they had different strongly typed datasets for each stored proc even if the underlying data was identical (ActiveEventsDataSet had the same fields as AllEventsDataSet). However, after initial generation, each dataset had been hand-tweaked over time so there was no single truth. Further, field names were never made consistant so it could be called 'eventName' in one dataset and 'name' in another and 'address' could have been used in both but for two different concepts.

    As to why the desire to put an object model in the middle, I don't honestly know the rationale. I'm as big an OO bigot as the next guy, but not when it comes to this much work to do it. As to why the switch from hierarchical to flattened... well, they originally were passing things over Remoting and thought the serialization hit must have been the source of the bottleneck (given the object model, this isn't necessarily unlikely).

    Of course, the mapping between dataset > object model and object model > object model was the best. Keep in mind, we own every piece of this code, only C# in the picture, and absolutely no need to abstract things like we did (no need for the remoting either)

    public void SetValue(string fieldName, object value)

    public object GetValue(string fieldName)

    I'll post the contents of those methods in a bit...



  • the anticipation grows. It's like a good book, just hope the ending doesn't disappoint!



  • do you intend to have a commemarative mug available at the end of this particular saga? : )



  • Is the WTF that he isn't going to finish this story?



  • [quote user="Fonzy"]Is the WTF that he isn't going to finish this story?[/quote]

    Shh! You're going to jinx it!



  • You poor guys on the east coast are going to miss part 3.  I just hope it's up before I go home tonight too.  I don't wanna be left hanging all weekend!



  • [quote user="Volmarias"][quote user="Fonzy"]Is the WTF that he isn't going to finish this story?[/quote]

    Shh! You're going to jinx it!
    [/quote]

     Sorry all. It turns out I don't have the code at home any more so it's going to have to wait until Monday.



  • [quote user="webzter"]...(given the object model, this isn't necessarily unlikely)....[/quote]<font size="+1">I</font>s that the same as is likely?



  • [quote user="triso"][quote user="webzter"]...(given the object model, this isn't necessarily unlikely)....[/quote]<font size="+1">I</font>s that the same as is likely?
    [/quote]

    No, because if I said it was likely then that would imply that I believed it to be so.



  • And.. we're back. Quick note, I was wrong on the method signature I mentioned above.. here they are, in all of their glory.

    Of the entire system, here is my favorite pattern.

    Every business entity implemented IPersistable and BusinessObject. Well, let's find out what IPersistable tells us:

     public interface IPersistable
     {
      // Property Access
      void SetAttribute(System.String name, System.Object value);

      System.Object GetAttribute(System.String name);

      // Contained Object Access
      void RemoveChild(System.String name, IPersistable element);

      IPersistable AddChild(System.String name);

      IPersistable GetChild(System.String name);

      // Business Object Name
      System.String ElementName
      {
       get;
      }
     }

    Hmm... well, that could set the stage for some fun. GetAttribute, SetAttribute, GetChild, AddChild... well, maybe it's not so bad. Let's check out BusinessObject.

     public interface BusinessObject
     {
      void Retrieve();
     
      void Save(ConnectionContext context);
     
      bool IsDirty
      {
       get;
       set;
      }
     }

    Nice, not a big WTF... but it's cool that BusinessObject doesn't follow the naming convention. It's an interface so it should be IBusinessObject. But, hey, at least we have an IsDirty. Personally, I would have changed it to IsVeryDirty, but that's just me.

    Let's look a bit more at how we can use those two spiffy interfaces. We're going to look at the Producer class.

      public System.String FirstName
      {
       get { return _firstName; }
       set
       {
        _firstName = value;
        IsDirty = true;
       }
      }
     
      public void SetAttribute(System.String name, System.Object value)
      {
       switch(  name.ToUpper() )
       {
        case "FIRSTNAME":
        case "FNAME":
         FirstName = (string)value;
         break;
        case "MIDDLEINITIAL":
         MiddleInitial = (string)value;
         break;
        case "LASTNAME":
         LastName = (string)value;
         break;
        //SNIP 30 lines like the above
       }
      }

    Whoa, wait... so, first, keep in mind this one simple fact. The only thing ever calling this code is other C# code. This is interesting. It's an attribute wrapper. The class has private fields, It then has an accessor around that. Then, just to make sure we're providing an easy way to hit this, we put a wrapper around those accessors. The brilliance is nice... take in anything, assume you know the type and hork it into the field. But, obviously there was some code mutation in there. Something started calling with "FName" instead of "FirstName". Well, the solution is simple, just provide another condition and you're golden.

    For completeness, here's GetAttribute:

      public System.Object GetAttribute(System.String name)
      {
       switch(  name.ToUpper() )
       {
        case "FNAME":
        case "FIRSTNAME":
         return FirstName;
        //snip
       }

       return null;
      }

    Well, that's kind of fun too. If you tell me you want the value of an attribute but you fat-finger the name, I'll just give you a null value back.. On the other hand, if you asked for the right attribute but it happens to be null, then I'll give you a null back. So, null is either a valid answer or an indicator of an invalid question.

    So, to review... I've just put a loosely-typed facade over a strongly-typed class, ask you to ensure you've properly typed what you're asking for, and then I might be nice enough to give you some runtime errors for things that could have been caught at compile-time. Or, I might not.

    Let's delve into  GetChild for just a minute...

      public IPersistable GetChild(System.String name)
      {
       IPersistable child = null;

       switch( name.ToUpper() )
       {
        case "COMPANIES":
         child = (IPersistable)Companies;
         break;
        case "PRODUCERADDRESS":
        {
         if ( ProducerAddress == null )
         {
          ProducerAddress = new Address();
          ProducerAddress.Parent = this;
         }
         child = (IPersistable)ProducerAddress;
        }
         break;
        case "PARENT":
         child = (IPersistable)Parent;
         break;
       }
       return child;
      }

    So, when you ask for the child, you need to tell it which child you want. And, child could be a single object, a collection, or the parent of the current object. If you had:

      Producer p = new Producer();
      //initialization stuff

    Then you could reference Producer p with:

      p.GetChild("ProducerAddress").GetChild("Parent");

    Frankly, I think that's a lot simpler than:

      p.Address.Producer;

    Plus, as an added bonus, you get a null back if you misspell anything.

     Ok, that does it for this post... we'll hit data access in the next one. Hopefully you can start to see how easy it was to trace which objects were dependant on which and how easy it was to track down runtime bugs.



  • so in short, they were re-inventing reflection with a half-assed interface?  and then using this pseudo-reflection throughout the code instead of compiler-verified statements?



  • [quote user="SilverDirk"]so in short, they were re-inventing reflection with a half-assed interface?  and then using this pseudo-reflection throughout the code instead of compiler-verified statements?[/quote]

    Looks like it to me.  And why would the author do this?  Either he didn't understand reflection, or he was just learning C#. 

    "You mean C# does that for me?  Wow, maybe that's why we're not using FORTRAN."
     



  • [quote user="SilverDirk"]so in short, they were re-inventing reflection with a half-assed interface?  and then using this pseudo-reflection throughout the code instead of compiler-verified statements?[/quote]

     Exactly... and, reflection has some great uses, but not when your in charge of the entire domain model where the model is highly cohesive.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.