Anit-pattern of March-- Found on Internet - The Internet can cure all of your problems

Anti-pattern of March Found On Internet

 

 

The elaboration says:

 

The practice of using code and techniques found on the internet without taking time to validate the source and ensure it is credible.

 



 This link provide some further reading on the anti-pattern ( including an interesting photo). for your convenience, I copied some here:

 

This anti-pattern refers to the practice of taking advice or code samples found on the Internet and immediately applying them to production code without taking the time to consider their effects. It's true the Internet is a huge source of knowledge, but it's also true that getting published on the Internet is not a particularly difficult task, and the peer review process is usually non-existent. Before trusting that a technique or code snippet found somewhere on the Internet, consider its source, and ideally try and apply it in a non-production environment first to ensure it does what you expect.

 

In my past 10 years working experience in software development business, I could say that I could not live without Internet, more specifically, I could not live without Internet search! Google created a new Internet time after World Wide Web became part of our life starting the second half of 90s. However, there is so much information on the internet, so much resource on the internet.  For the same topic you can find articles with total opposite opinions. logically, at least one of them is wrong.

 

One would asked “are we going to stop searching on internet?”  absolutely NOT. The key point here is “validation”. First of all, you want to ensure it is from reputable source or it is endorsed by reputable party. If it is from a personal blog, you want to take extra  effort to verify it. For “open source” solution, you want to get the source code, and verify the source code before putting it in production system. Accurate test, security test, reliability test need to be conducted on these “open” products before use them in commercial software development.

 

When you are very excited about something you found on internet, that is the time you need to sit back and do some homework before jump on your feet.

using HTTP Client to interact with Restful service created with ASP.Net WebAPI

Refer back to the article on how to implement CRUD operation using WebAPI. Today I am going to show you the other side of the story.

 

The code is surprisingly simple, let me show you one by one:

 

1. Create

 

public async Task<bool>CreateAsync(Person person)

        {

            HttpResponseMessage response = await PostAsJsonAsync<Person>(ServiceUris.ControllerBaseAddress, person);

            return response.IsSuccessStatusCode;

        }

 

2. Read all

 

        public async Task<IEnumerable<Person>> GetAsync()

        {

            var response = Get(ServiceUris.ControllerBaseAddress);

            if (response.IsSuccessStatusCode)

            {

                return await response.Content.ReadAsAsync<List<Person>>();

            }

            else

            {

                return null;

            }

        }

 

3. Read one

 

         public async Task<Person> GetAsync(int id)

        {

            var response = Get(UriAddress(id));

            if (response.IsSuccessStatusCode)

            {

               return await response.Content.ReadAsAsync<Person>();            

            }

            else

            {

                throw new HttpClientException(response);

            }

        }

 

4. Update

 

        public async Task<bool> UpdateAsync(Person person)

        {

            var putUri = UriAddress(person.Id);        

            Debug.WriteLine(putUri);

            HttpResponseMessage response = await PutAsJsonAsync<Person>(putUri, person);

            return response.IsSuccessStatusCode;

        }

 

5. Delete

 

public async Task DeleteAsync(int id)

        {

            await base.DeleteAsync(UriAddress(id));

        }

 

 

The constructor of the class is looks like the following:

 

 

public PersonManager(string baseAddress)

            : base(baseAddress)

        {

        }

 

I have a few unit test cases for it, let me show you some:     

 

 

 

        [TestMethod]

        public void PersonManagerGetAsyncTest()

        {

            var target = new PersonManager(TestSettingsLocal.ServiceBaseAddress);

 

            var result = target.GetAsync().Result;

 

            Assert.IsNotNull(result);

 

            foreach (var p in result)

            {

                Debug.WriteLine(p.ToString());

            }

        }

 

        public void PersonManagerGetAsyncByIdTest()

        {

            var target = new PersonManager(TestSettingsLocal.ServiceBaseAddress);

 

            var result = target.GetAsync(1).Result;

 

            Assert.AreEqual("Peter", result.FirstName);

            Assert.AreEqual("Lu", result.LastName);

 

        }

 

You might notice that the code here is very simple, it does not even have HTTP related code. Well the code here is simple is because there is a base class does all the heavy lifting behind the scene. This is how the class is defined.

 

 public class MyServiceProxy : ProxyBase

{

}

 

 

For these who are interested to see how ProxyBase is written, please drop me an email, I will send you the source code for the base class.

 

 

 

Web Configuration Transformation and Web Project Deployment

I thought to share something I first learned back in 2011 when I prepared for Microsoft certification examination for ASP.Net 4.0.  latterly I put it in use in my project. Since I put it in use, I start to appreciate it more. I could not think otherwise how could I do it without significant development effort. (of course, nobody wants to write code if we do not have to!). It has some relevancy to what we do in our project and if you are using VS2010 or above or thinking of using VS2010 or above for your Web app, whether it is classical ASP.Net form or MVC, you will find it interesting. Especially those who are involved in build/ deployment or tokenization process might find it useful or at least you will have an alternative to what you are using now in designing deployment strategy Visual Studio 2010.



ok, let me get it started…



Before Visual Studio 2010, ( like Visual Studio 2008 we are using), when you create an ASP.Net Web Project, IDE will add a Web.Config file to your project. if you want to have different setting in different regions ( Test, Staging or Production), you need to replace the region specific setting in the web.config file yourself. Some do it manually, some create some kind tokenization process to handle it. In order to deploy the project to a server you need to add a Web setup project in your solution and configure it to make a MSI file then copy the MSI file to the server you want to install and install the MSI package there.



This scenario is not ideal by all measures. If you are doing it manually, you tend to make mistakes as humans always do, if you are doing it using a tokenization tool, you will have one more step to worry about… as for MSI file installation, there are many shortcoming associate with it  If you want to use the built-in publishing process, you will find difficult to do the tokenization process.



well, starting with Visual Studio 2010, Microsoft introduced a process call “Configuration Transformation”.



After a Web project is created, you will see not only Web.config file, but also 2 files under it. please refer to the screen shot below:

 The 2 new files you did not see in Visual Studio 2008 are Web.Debug.config and Web.Release.config file.  “Debug” and “Release” is directly link to the default “Build Configurations” IDE created for you, Debug mode and Release mode. If you want to created more Build configuration using “Configuration Manager”, you will be able to add new “Config Transformation” for the new build configuration you created.  As you can see in the following screen shot, after I created a new Build Configuration, “Staging” I can add Config Transformations, and after I do  that I got one more file created

Let’s say in your project you have a DB connection and a setting, which will be different in different region.. you will most likely have your web.config file like the following:

<connectionStrings>

    <add name="ApplicationServices"

         connectionString="data source=.\SQLEXPRESS;Integrated Security=SSPI;AttachDBFilename=|DataDirectory|\aspnetdb.mdf;User Instance=true"

         providerName="System.Data.SqlClient" />

  </connectionStrings>

  <appSettings>

    <add key="BaseAddress" value="http://localhost:3593/"/>

  </appSettings>



then you will have the same setting but different values in these Config Transformations.



Release:

<connectionStrings>
    <add name="ApplicationServices"
         connectionString="Data Source=ProdServer;Initial Catalog=ProdDB ;Integrated Security=SSPI;"
         providerName="System.Data.SqlClient" />
  </connectionStrings>
  <appSettings>
    <add key="BaseAddress" value="http://ProdServerAddress/"/>
  </appSettings>


Debug:

<connectionStrings>
    <add name="ApplicationServices"
         connectionString="TestServer;Initial Catalog=TestDB ;Integrated Security=SSPI;"
         providerName="System.Data.SqlClient" />
  </connectionStrings>
  <appSettings>
    <add key="BaseAddress" value="http://TestServerAddress/"/>
  </appSettings>


Staging:

<connectionStrings>
    <add name="ApplicationServices"
         connectionString="StagingServer;Initial Catalog=StagingDB ;Integrated Security=SSPI;"
         providerName="System.Data.SqlClient" />
  </connectionStrings>
  <appSettings>
    <add key="BaseAddress" value="http://StagingServerAddress/"/>
  </appSettings>

But if we stop like that it won’t work. This is the time to introduce transformation process.  just pick up any of these transformations as an example: ( let’s say I pick Release), I need to add some transformation code in the transformation file:


Release:

  <connectionStrings>
    <add name="ApplicationServices"
         connectionString="Data Source=ProdServer;Initial Catalog=ProdDB ;Integrated Security=SSPI;"
         providerName="System.Data.SqlClient"  xdt:Transform="SetAttributes" xdt:Locator="Match(name)/>
  </connectionStrings>
  <appSettings>
    <add key="BaseAddress" value="http://ProdServerAddress/" xdt:Transform="SetAttributes" xdt:Locator="Match(key)"/>
  </appSettings>

What does it mean here? for the first one, it means go to the Config file and look for any connection string setting with the name of ApplicationServices and replace all its attributes with the ones here when the build mode is Release build. The second line means look for a setting with the key of   BaseAddress and replace all its attributes with the ones here. ( of course, you would not want to hard code id and password here, instead you want to use integrated security for database security, but you get the idea here, right?)

Okay, with all these set up, your build and deployment is just a few button clicks.

You can choose “Web Deploy” File System Deploy” or “FTP Deploy”. For my case, I use “File System Deploy” for on-promise server deployment, and I use “Web Deploy” for Azure web site deployment.  For Azure Web site deploy, all you need to do is load the Publish Profile you downloaded from Azure portal and you are all set. For different region, you just choose different Build Configuration.  

When you are doing Web publishing, the IDE is smart enough to detect the versions of the affected components and only push the components that have been changed. The preview function allow you to see what will be deployed to the site.  This greatly reduced the time spent and the risk associated with deployment process. The following is the screenshot of the preview screen.

once you are happy with the preview you see, all you need to do is click on <Publish> button. the IDE will make appropriate build based on your selection, transform the web.config file with the transformation you have and published the bits to the site. at the end of it, it will launch IE with your site opened in it. For my site, with the size of 50MB, the publishing process takes 1 to 2 minutes if it is fresh build. If it is a update build, it just takes sub-seconds. Of cause, the IDE needs to build the solution first. In my experience, I find it is better to do a “Rebuild All” before publishing would save you some time, in case the build produce problems.  



This could potentially replace the tokenization process we use if we decided to upgrade to VS 2010 or above.  Please refer to the links below for further reading on Web Configuration Transformation



http://msdn.microsoft.com/en-us/library/dd465326(v=vs.110).aspx

http://msdn.microsoft.com/en-us/library/dd465318(v=vs.100).aspx

If you are on “Standard” tier in Azure, you are offered backup restore functionality for your web site. with that I can backup your site  before deployment, and in case deployment encountered problem, you can  easily rollback to the one you had before your deployment process. From the screenshot below you can see a link with the caption of <BackUPS>

As always, feedbacks and comments on this article and suggestions on future writing are welcome.

MVC and MVVM with WPF

In any MVC pattern, there is no argument about the definition of the view. View is the user interface that user interact with the software system. In classic window app, it is the Window form, and controls. In classic ASP.Net app, it is the ASPX page. In MVC.Net it is the ASPX page made up by HTML and other tags.

In WPF app, it is the XAML file. In another word, it is the UI.

It is commonly accepted that UI layer should be free of business logic, including user process logic.

So what is Model?  Model is not only Business Entity object model. It is the business logic of the system. In a typical 3 tier architectural system, most of business logic should be implemented in the service layer.  Then the question left unanswered is “then what is the rest?” In general term we call it user process logic. In MVC pattern, it is the controller, in MVP pattern, it is the Presenter. In MVVM pattern, it is the ViewModel.

Through service proxy, the business object model, business service (in WCF world, they are represented by data contract and service contract) are made available as part of model. The rest of the model is the user interaction logic or user process logic.

It is commonly acceptable that it is our goal to make User Interface Process (UIP) logic agnostic of UI platform and independent of View. This is so for many reasons. Reusability is one of them, testability is another.

Within MS platform, there are 2 types of application, window app and web app.

Before MVC.ASP.Net for Web, WPF for window, all we can do is to minimalize such dependency we could not totally eliminate it.  In either classic ASP.Net or classic window form, there is no way to totally eliminate code-behind logic. And these left over at the code behind are most likely the user interface process (UIP) logic.  Any code that dependents on the UI elements would be regarded as dependent of UI elements.

Thanks to two-way binding provided by XAML. With XAML, we finally are able to achieve the goal of making UIP logic independent of View.  The bridge between the back end service and UI is ViewModel. The bridge between the View and ViewModel is two-way binding

From component reference point of view, UI (View authored in XAML) reference ViewModel, ViewModel reference the model provided by the backend service through proxy.

Between View and ViewModel there are 2 ways to guaranty the reference is single direction. The first way is to make them in separate components and make the UI reference the ViewModel. In this way, ViewModel is forced to have no knowledge of the View.

Another way is to ensure that all the UI element in XAML have no id, no name. So that ViewModel has no way to reference them.  In both cases, make sure we do not have code behind the UI elements written in XAML.

The 2 success factors in WPF MVVM application are 1) correctly employ of the pattern. Another is XAML authoring. One could use both Visual Studio and Expression Blend to author the UI.

For the rest of this article, I am going to focus on ViewModel aspect of MVVM.  However, I would like mention that there is lack of study material on XAML authoring and it is hard to find some good XAML design guys in the market.

With Reference to ViewModel, the UI designer could design UX with dummy data in design model within either Visual Studio or Expression. However, Expression Blend is more designer friendly while Visual Studio is more developer friendly.

The wonderful news is that with WPF + MVVM, designers and developers can work on the same codebase using the tools they are familiar with. 

Fix it once, fix it well and fix it for good - Defect filing and defect interpretation

In my code review, I came across a change set. In the change set,  we added event handlers to adjust the edit value of the control from “0” to String.Empty. The following is the code:

Private Sub BasePriceEmptyWeight_CustomDisplayText(ByVal sender As Object, ByVal e As CustomDisplayTextEventArgs) Handles txtBasePrice.CustomDisplayText, txtEmptyWeight.CustomDisplayText
            If e.Value = "0" Then
                e.DisplayText = String.Empty
            End If
End Sub

After some research, I found that the controls are bound to a properties in  VehicleDetail named BasePrice and UnloadedWeight.

 Me.txtBasePrice.DataBindings.Add(New System.Windows.Forms.Binding("EditValue", Me.VehicleDetailsBindingSource, "BasePrice", True))

The following is the code on how they are defined:


Public Property BasePrice() As Double
Public Property UnloadedWeight() As Integer

and the respective column in the database is nullable numeric

[BASE_PRICE] [numeric](9, 2) NULL,
[UNLOADED_WEIGHT] [numeric](9, 0) NULL,

the defect says when I do not have a base price or UnloadedWeight and do not need them, I do not want to see their value as 0.

well, that would be difficult, because Double/integer are value types , even you assign Nothing (null)  to it, it will have value zero. so we came out with the idea of faking the value in UI to turn zero to String.Empty.

With this solution, the user would not see zero in the UI, but internally, the value of the base price is still zero. In the database, zero still get to be written into database even it is a nullable column.  Besides the confusion it would cost in understanding data and  understanding the code, ( one would ask the data source is having numeric type, why we change a valid numeric value to a non-valid numeric value? what about “00” or “000”?)   I was wondering how many UI places we need to fake?  This one defect need to be fixed in how many places ? Do we know  where are they?  How many rounds we need to go through before we can claim we have indeed fixed this defect.

I thought the proper solution to the defect could be changing the type of BasePrice /  property from Double to Nullable Double and change UnloadedWeight from integer to nullable integer.  when their type is changed, in the entire system, when their value is nothing,  The binding source will automatically put nothing on the control instead of zero. and when the object is persisted to database, Null get to be persisted to the database table.as well.  This defect would not be reopened or created because in some other places we missed. Granted, this might take a bit more effort to do, but when it is fixed once,  it will be fixed for good.

I thought there are technical and economical motivation for us to fix the root cause of the defect instead of proving quick fix as we did. It might be acceptable if we do that in production support branch, but it would not be a good choice in product development branch. We might have some justifications for doing what we did in this case, but in general, I would say it is not encouraged in software development project.  I am here calling upon all of us : Fix it once, fix it well and fix it for good when it comes to defect fixing.

After the above message was out, over the weekend, I got an email asking me the impact of the suggested solution.  Well,  it would be difficult for me to imagine that in one place we do not want to see zero, and in another we want to see zero. there must be some kind of standard treatment within the application. whatever it is impacted by the suggested changes, that’s where we should laid our eyes on.  There is no magic wander in software development business.

However, I would think this is not completely the responsibility of the development team, the QA team may have its share as well. if the defect is filed as following:

the business requirement of the system indicated that when Base Price is not applicable, the system should not display zero for base price. but in this specific screen, we see zero in such scenario. which need to be fixed.  

In this case, the defect described a general business rule and cite a specific case where the rule was not followed. It indicates that this is case where the rule is broken, the system not only need to make sure in this case the rule is not broken, but also need to ensure the rule is not broken in the entire system. so when we fix this defect, we not only make sure the system acts as designed in this case but also need to ensure it acts correctly in all cases.

However, if the defect was written in another way, the interpretation would be different.

In this screen, I see zero for Based Price when Base Price is not applicable, which is wrong, please hide zero in the screen.

First, it did not describe general business rule or business requirement, it rather focus on this specific screen and specific scenario. it seems suggesting everywhere else is good, and just this place we have a small problem. When the defect is written in this way, it would be very attempted for the development team to do what it is done for this case .  However, if our development team aim to  go above and beyond, we may want to ask back, how about other scenario, what is the general business rule with regarding to this piece of information.


Take another example here:

Let’s say I was  given a task to write an function to calculate  f(x, y) = x power of y. (for example, when x = 1, y = 1, you get 1, when x = 2, y = 2 you get 4 and so on,,,)
but I have returned the math Learned in the college or high school to my teachers, I did not get what is power means here. but I have to finish the work assigned to me. looking at the example data, I figure it is just about x + y. maybe Power is just a fancy word here. so I did.

Not soon after I claim have completed the function, I got a defect , the tester says when I put 2 and 3 in, I got 5 , which is wrong, the right answer is 9.
I thought , it looks like in all other cases the function is doing what it is supposed to do, but only when x=2 and y = 3, they want to see 9. instead of 5, then I quickly jump into the code and add an if statement in it.

if x = 2 and y = 3  return 9 else return x + y;

Going on in this way, many defects got filed and got quickly fixed. the code becomes a long haul of if statement and at final else we still have  return x + y.

Now, you are coming to the picture,  you ask me: Peter,  we may be able to take out these big if-else and just to return  Math.Pow(x,y). That does not sit well with me. First of all, you mean all the hard work I have put in for months can be replaced with one line? secondly I am very nervous about the suggestion,  the suggested change impact lots of cases. I might need to do lots of testing and maybe I could create more defects…   Do you have some sympathy for me? if so, please comment on this article to show your sympathy, that would offer me great comfort…

speaking of “faking”  I came across another case this morning, I am attempted to share it with you.

There is a defect say when a transaction is loaded from database, a flag property IsSelected is not set. To fix the problem the code change add a line of code in the Data Access Layer method  which is used to load the Entity from the database.

MyEntity.IsSelected = True.

Quick and sweet, the defect got fixed! now the check box in the UI started to show up… I pat myself on my back, job well done!

Is it true? do we know if the user checked that checkbox before the transaction is saved to Database ? Do we know when the data is saved to the database, is the the property persisted to database or not? do we know which column it is saved to do? do we know in the same Data Access Layer method we touched in the code, does the stored procedure retrieve the data from that column?  Is this the problem in a Data Access Layer method when the transaction is saved to Database or is it the problem when the transaction is retrieved form the database? maybe there is no such column in the table to hold this piece information at all?

Are we making the system better than it was before in the way we fix defects? By doing what we are doing, when can we get the system completed and accepted with some kind of reasonable quality standard that any software product should have? I have more questions than answers.


P/S: I wrote this post originally on November, 2013, it is until today I realized that I forgot to publish it... so I review it once more and publish it today.

Root cause and exception handling

I have a story to tell, this is how it goes:

There was a boy and his mother. One day, the boy came back home with blood on his forehead. Yeah, he hurt himself on his forehead.

The mother asked “ what’s happened, my dear?” “I bend my head on the door when I go potty.”  answered the boy.
“what? silly boy. you should open the door and then going through it. why you bend your head on it?”

“ why? the door supposed to be opened all the time!” augured the boy.

“My dear boy, this is what you want to do next time.” said the mother with smile, “no matter what is supposed to be, you check it to see if the door is open before you going through it..”
“thank you mom, I will do as you say.” replied the boy.

The door supposed to be open but it is not. that is the root cause of the boy getting hurt. but the month did not address the root cause , rather make her son smarter so that he won’t get hurt.

The next day, the boy came back with his pants wet. The mother asked “my son, what happened, did you go the bath room to pee?” . “No, the door is closed.” replayed the boy sadly.
“My dear boy, this is what you want to do,” said the mother with simile. “ when the door is closed, try to open it.”

The root cause is still the same, the door is not open and it supposed to be. but the mother did not address the root cause of the problem, rather make her son smarter to open the door when he need to go potty

The next day, the boy came back with his pants wet again. The mother asked “my son, what happened, did you try to open the door of the bathroom “. “I did, but the door was locked.” replayed the boy sadly.

“My dear boy, this is what you want to do,” said the mother with simile. “ when the door is locked, report you teacher and seek her help..”. “thank you, Mother, I will do as you said.

From the surface, the root cause is the same,  as the boy was told to open the door when it is not open, so the boy try to open the door, hence he found the door not only closed but also locked. should he did not try to open the door, there would be no way for him to know for sure, the door is locked. all he could say is “the door may be locked, maybe not. I do not know”  with the newly developed intelligence, he managed to identify a more precised root cause.  but the boy was not taught on how to handle the root case “door is locked”.  His mother did not call to the childcare center to ask them to fix the root cause. instead, she taught her son another smart thing to do: when you are not able to handle it, report to someone who can.

The next day and the days after, the boy came back from the childcare center without incident.

From the beginning to the end, the mother did not do anything to directly address the root cause. From “door is not open” to “door is locked”  all she was doing was teaching her son to be more intelligent in handling the environment. When the precise and accurate information was provided to the relevant party, The root cause will be addressed or at least, the boy would not get himself hurt or get his pants wet. Even if one day, the door was really locked and the key is nowhere to be found, the teach should and will find some other ways to help the boy to pee. if not, the mother will really have a complain to made…
 

Now, coming back to our programming work, we, as programmer, like the mother, the program we write is like the boy. The program will do as we told it to do, Nothing more and nothing less. Based on the logged unhandled exception. instead of immediately doing something with the goal of addressing the root cause. we tell our program to be smarter. when we see exception of “Object reference not set to an instance of an object.” it is like the mother see blood in her son’s forehead.

When you see exception of “Enum value '0' is invalid for type 'CustomerType' and cannot be serialized.” it is like the mother see the boy with his pants wet…  You do not want to see these types of exceptions logged as inner exception while the outer exception is

The information entered does not match the information on file. Please call the Department of XYZ  at (800) HLP-DESK (457-3375) for assistance or Please visit a ABC office for assistance.

As the development resource (like the mother). it is our job to make the program we write more comprehensive and more robust  in handling various situations in the running environment. When we do that, the root cause will be solved by the respective party, or at least we won’t  get blamed for the problem caused by the root cause.

check, mitigate and report are the 3 steps the mother taught her son. These are also the same strategy we could employee when see some unhandled exceptions in the system..

Bad Smell in code - lack of constructors (Object reference not set)

For some people who are not too much into programming, you may not know much about constructor, but I am sure if you are in this project for a while, you surely know something about errors with the message of "Object reference not set". Most of "object reference not set" are caused by lack of constructor, the rest of it is caused by "Nothing", in my article titled "Nothing, a necessary evil" I addressed of usage of "Nothing" ( null in c#) issue, in this article I am going to talk about constructor.

Before proceed into the detail of constructors, let me show you some statistics on "Object reference not set" exception:

In production system, from July-27-2011 to September 10th 2013, Just in the Web presentation layer alone, we recorded 58476 exceptions. Of which 14458 is "Object reference not set". Excluding 4080 repeated ones, "Object reference not set" error contributes 26% of all exceptions logged.

now, let's look at the codebase we have,

Just take DataEntity as an example, there are 1137 classes in the project, among which 890 classes do not have any constructor. which means 78% of our entity classes do not have even one constructor. within 22% of the classes with constructor, some of them are empty ones. like the one in AddressBE, AuditEventBE, AuditTransactionBE, BatchCPCOutOfStateLetterBE, BatchErrorRecordsBE, PINRenewalUpdateRequestBE, MetaDataBE, BusinessTableBE... the constrcutor is something like

Public Sub New()
End Sub

I am sure you can find lots more than I did here.


so the real % of classes lack of constructor could be more than 80%.

Now let's go back to our code to see how we cope with this "lack of constructor" smell. you guessed it right! we check if it is Nothing before access it"

If batchRequestErrors IsNot Nothing Then
If batchSalvageAgentExist Is Nothing Then

Just in BO project alone, we have 856 "IsNot Nothing" checks and 429 "Is Nothing" checks.

There are total 40291 line of code in BO project. Based on the figure here, we could calculate that there are 3 "Nothing" checks in every 100 line of code. When there is an "Object reference not set" defect filed, we find the spot and add a Nothing check around it... This is not proper way of fixing "Object reference not set" defects. Instead we should find out the source of "Nothing" value, and find out if it a valid Nothing or invalid Nothing. Does the class have a constructor ? Is the constructor failed to instantiate the instance?

With the way we do, we end up with 2 bad smells in our code and one bad taste in user experience. In the code, we have "lack of constructor" and "excessive of nothing check". in user experience , we have many "Object reference not set" STEs in the log. I am sure you want to do better than that!

Fortunately, the fix to this bad smell is magically simple: Add a default constructor to all classes, and in the constructor, call its base class constructor and insatiate all its members. The following is an example:

Public Sub New()
Mybase.New()
Me.mID = 0
Me.mVehicleID = 0
Me.mDescription = String.Empty
Me.mVehiclePurchaseDate = Date.MinValue
Me.mRTSSurvivalshipCode = String.Empty
Me.mRTSSurvivalshipDesc = String.Empty
Me.mIsUnderCover = False
Me.mIsOrConjunction = False
Me.mTitle = New TitleBE()
Me.mRegistration = New RegistrationBE()
Me.mVehicleCost = New VehicleCostBE()
Me.mOwnerships = New Collection(Of OwnershipBE)
End Sub


2 years ago, I identified this bad smell and I published a guideline on usage of constructor... along the way of my code review, I personally added lots of constructors to the classes. I thought to put together this article to refresh the call of using constructor.