So, why are statics bad?

I’m a huge fan of automated testing. I started with unit tests, but have recently moved into the realm of behavior tests. Yet, for any automated testing, you have to have good architecture in your code to ensure that you can test in any environment.

Let me give the most basic example: the use of a logging facility. Take for example the following loggger that I see all too often:

{% codeblock lang:csharp %} public static class LoggingHelper { public static void LogMessage(string message, string area) { var logPath = Constants.LogPath; using (StreamWriter writer = System.IO.File.AppendText(logPath)) { writer.WriteLine(“INFO\t[” + area + “]” + message); } }

public static void LogError(string message, string area)
{
    var logPath = Constants.LogPath;
    using (StreamWriter writer = System.IO.File.AppendText(logPath))
    {
        writer.WriteLine("ERROR\t[" + area + "]" + message);
    }
}

public static void LogException(string message, string area, Exception exception)
{
    var logPath = Constants.LogPath;
    using (StreamWriter writer = System.IO.File.AppendText(logPath))
    {
        writer.WriteLine("EXCEPTION\t[" + area + "]" + message + " - " + exception.Message);
        writer.WriteLine("Stack Trace" + exception.StackTrace);
    }
}

} {% endcodeblock %}

Here are a few areas the above code block can improve:

• There’s no exception handling. At all.
• There’s no mutexes/resource locking. Thus, it’s possible for multiple threads to attempt to open the file at the same time. The first one in will succeed. All others will throw exceptions.
• It’s insanely duplicative. In fact, I was able to copy every line of LogMessage to LogError and only have to change 2 lines of code. The same goes for LogException, where I also added an additional line.
• What happens when I have another type of message I need to log? That means I must add another method - 6 of those lines likely the same as LogMessage.
• What happens when I need to change the location of the log file?
• What happens when I need to change the underlying facility for logging? Say, from files to databases?

The last two are key here. For the first, I can change from Constants.LogPath to something like ConfigurationManager.AppSettings["LogPath"], But what happens when the file is a network file system? Not accessible via standard UNC paths? You’re up a creek and this entire class needs to change. That’s pretty much the case of the second question.

You should assume that your underlying logging destination can and will change at any time. If you think that the simple solution to this is to simply change the LoggingHelper class, think again. Let’s take this further. You should assume that your destination will be different for multiple scenarios. In enterprise development, this is frequently the case. In a pre-integration environment, you may only want to log to the filesystem because it may be a single server. However, in integration and production you might choose to log to a database since you’re deploying to a farm of servers and writing to the filesystem makes diagnosing problems a true headache.

Back to the problem of automated testing, you don’t really want any logging. Why? What happens when your test runner doesn’t have access to the logging path? What happens when your test environment doesn’t have the path at all? What happens when you forget you have your test environment dumping massive amounts of logs to the filesystem only to go on a goose chase figuring out why your builds randomly started to fail due to the disk being full? In the case of unit testing, /dev/null should be your destination.

Let’s consider one last secenario: what happens if you want to log to multiple facilities at once?

In any of those cases, using a static class with static methods does not afford you any flexibility. If you want to expand to multiple logging facilities, you would be forced to create another static class. That class is now called in addition to this one. Yuck.

Here’s where I come back to the SOLID principles. Let’s look at how Static classes (and static methods) violate these principles.

• Open/Closed Principle - Classes should be open for extension, but closed for modification.
  • With statics, you cannot extend and override the behavior. You throw out the idea of polymorphism. This makes it very difficult impossible for me to inject different behaviors for whatever need I may have.
• Dependency Inversion Principle - Depend upon Abstractions. Do not depend upon concretions.
  • With statics, every class that leverages its facilities are now dependent upon a concretion. There is no way to pass around an abstraction of a static class or regular class with static methods. Your only way of doing this is with reflection and that’s a definite code smell.

In summary, because I cannot leverage the power of inheritance, polymorphism, or really any aspect of object oriented analysis and design with statics, it feels a lot like procedural programming. I suggest leaving this practice behind.

So, why are helpers bad?

My discussion with my colleagues yielded an interesting observation: everyone’s opinion of a helper was slightly different. One thought of it as a single data access layer. Another thought of it as a way of providing consistent authentication for data access. But others think of helpers as the giant “I don’t know where to stick this method” bucket. So, you end up with something like this (methods only, since that should convey what I’m getting at):

{% codeblock lang:csharp %} public static class Utils { public static string CleanInput(string input);

public static void FlushCacheIndex();

public static bool CheckIfSystemIsHealthy();

public static int GetCountOfActiveUsers();

public static MyModel CreateModelFromInput(string name, string email, int age, DateTime birthday);

} {% endcodeblock %}

These types of helpers are a maintainability nightmare. For starters, if you haven’t read my section above on why statics are bad, step back and read it. Both SOLID principles are still violated. Yet, there are also a couple of others that are violated.

• Single Responsibility Principle - A class should have only one reason to change.
  • It’s clear in the case above that there are multiple reasons this class exists. However, there are other helper classes (such as StringUtils which are not as clear. However, I’m still a firm believer that the Open/Closed Principle dictates whether to add yet another method to an existing class, or create a separate class. I’m a huge fan of small classes as their much easier to test and maintain.
• Interface Segregation Principle - Many client-specific interfaces are better than one general-purpose interface.
  • Okay, so this isn’t an interface. ISP is definitely meant for solving another problem. However, the basics behind this principle apply here. This is a general-purpose class. Why should I have my code strongly-coupled to this class if all I need is one method within it?

Now, to tackle some of the other arguments in favor of helper classes, let’s look at another example I’ve seen in the past:

{% codeblock lang:csharp %} public class DatabaseHelper { private static DataAccessObject daoInstance;

public static DataAccessObject CreateDataAccessObject(User invokingUser)
{
    if (daoInstance == null)
    {
        var dao = new DataAccessObject();
        dao.AuthenticationMode = AuthenticationModes.Shared;
        dao.Credentials = new DataAccessCredentials(invokingUser.Kerberos, invokingUser.Token);
        return dao;
    }
    return daoInstance;
}

} {% endcodeblock %}

Ah, rather than a helper, you meant a factory. Okay, I can dig that…sorta. We’re still using static here, which makes it impossible for me to factorize the creation of my DAO without strongly coupling my business logic to this specific class. Now, a factory is meant to abstract away the details of creating an object of a specific type. There’s some generally common expected inputs, and you get a generic implementation at the output. But, how should you go about doing that?

If you’re using a proper dependency injection/Inversion of Control container, you’ll be able to configure your application instance to inject a provider which implements the following interface:

{% codeblock lang:csharp %} public interface DataAccessFactory { DataAccessObject CreateDataAccessObject(User invokingUser); } {% endcodeblock %}

Furthermore, if you’re in a spot where you need a singleton instance of the DataAccessObject, then you can make sure that you implement the appropriate logic in your Factory instance and setup your IoC container to maintain a singleton instance of the object.

When discussing the use of “helper” classes, it’s more difficult to outright say they aren’t useful, because each person’s definition of helper classes are different. Nevertheless, if you venture down the path of bucket “utility” classes, stop. There are better approaches you can take.

Well, I still don’t agree with you…

That’s fine. You’re entitled to your opinion. Just don’t expect for me to pass your code reviews.

Are there circumstances where the use of a static method might be worth it? Quite possibly. But I rarely encounter those cases. If one of those reasons is “it’s faster to do it with a static than via good architecture” then you’re just being lazy. Yes, software development is about delivering value. However, what debt you introduce to the system must later be addressed save you be unable to deliver any value once you’ve built a castle with no doors around you.

I’m not alone in this. Check out just a few other blogs which carry several really good arguments against the use of statics or helpers:

• Anti-patterns and Worst Practices - Utils Classes
• Util Classes Must Die
• Are Helper Classes Evil?
• Helper Classes are Evil

Where did you shape your thoughts on best practices?

In my previous role at Improving Enterprises, there were a number of really talented technologists, passionate about best practices. They were staunch supporters of user groups and community as well. Thus, my opinions on best practices have been shaped not only through previous co-workers and colleagues, but also by community.

Community has a lot of benefits, and has a lot of diversity of thought. The software community provides an atmosphere for technologists to actively try many of the best practices they preach through katas, discussions, pull requests, and many other channels. These best practices are most commonly focused around the use of patterns. Why reinvent the wheel when patterns exist to help solve the same or similar problems? However, these patterns often have justifiable use: they leverage the facets of object oriented languages in the manner they were expected to be used.

This is where I started to learn about best practices. My source of community was extremely diverse. It included my colleagues at work, my close developer friends, and those technologists in the community who were extremely passionate about their work. Some of these technologists I engaged only through user groups, some through their blogs, and some through discussion boards. I am a high achiever, and in order for me to achieve, I must learn - so I am always on a continuous search for new information - even if it contradicts previous thoughts or practices I held imporant to me.

Finally, the only other way to really learn and solidify the importance of a best practice is to, well, practice it. With enough practice, you’ll invariably run into other practices which box you into a corner or make your life very difficult. The more times you encounter these situations, the greater a believer in best practices which leverage the power of object oriented design and development.

Diversity of thought is a great practice and should be encouraged. It leads to more out-of-the-box thinking, approaches to problems, and ensures that all risks are vocalized. However, one of the most important things I’ve had to learn is that no matter what, diversity of thought should always be supported with sound reason and justification. Simply citing a preference to a mode of practice, idea, or solution without good justification to a practice is hard to accept in a professional setting.

[2] http://www.improvingenterprises.com/ [3] http://en.wikipedia.org/wiki/SOLID_(object-oriented_design)

• My Site Instantiation Interactive Request Queue
• My Site Instantiation Non-Interactive Request Queue
• My Site Second Instantiation Interactive Request Queue

The problem is, those timer jobs were missing from my server!

I checked for the timer jobs first through Central Administration, and then subsequently thru PowerShell. The following are the results I found:

Get-SPTimerJob | sort Name | ft Name

{% img center /images/posts/2013-08-02-sharepoint-2013-my-site-timer-jobs-missing/01-missing-timerjobs.png Timer Jobs Missing %}

I decided to check for the My Site timer jobs on a know working SharePoint 2013 farm. Sure enough, they were there:

{% img center /images/posts/2013-08-02-sharepoint-2013-my-site-timer-jobs-missing/02-present-timerjobs.png Timer Jobs Present on Farm %}

I wanted to look for a way to re-register timer jobs. There should be no reason for me to do something drastic like uninstall/reinstall the User Profile Service just to get the timer job re-registered. This timer job, as expected, was in the Microsoft.Office.Server.UserProfiles assembly:

{% img center /images/posts/2013-08-02-sharepoint-2013-my-site-timer-jobs-missing/03-timerjob-definition.png Timer Jobs Definition %}

So, I cracked open dotPeek and decided to hunt for that timer job definition and find who was using it. It turns out, there’s a web application-level feature (MySiteInstantiationQueuesFeatureReceiver) which registers these timer jobs:

{% img center /images/posts/2013-08-02-sharepoint-2013-my-site-timer-jobs-missing/04-featurereceiver.png Timer Job Feature %}

There is a public static method inside that feature receiver used for registering the timer jobs.

{% img center /images/posts/2013-08-02-sharepoint-2013-my-site-timer-jobs-missing/05-methodtoregistertimerjobs.png Method to Register Timer Jobs %}

So, I had one of two options: invoke the static method directly, or simply toggle the feature. I opted to toggle the feature:

$feat = Get-SPFeature 65B53AAF-4754-46D7-BB5B-7ED4CF5564E1
Disable-SPFeature $feat -Url http://webapp
Enable-SPFeature $feat -Url http://webapp

{% img center /images/posts/2013-08-02-sharepoint-2013-my-site-timer-jobs-missing/06-toggle-feature.png Toggle Feature %}

Finally, I double checked and verified the timer jobs were now present!

{% img center /images/posts/2013-08-02-sharepoint-2013-my-site-timer-jobs-missing/07-timerjobspresent.png Timer Jobs Present %}

I then went to the My Site host URL and got into the queue for provisioning. Within 5 minutes, my personal my site was provisioned!

{% img center /images/posts/2013-08-02-sharepoint-2013-my-site-timer-jobs-missing/08-mysite-provisioned.png My Site Provisioned %}

{% codeblock %} The target of the operation was not found. Internal type name: Microsoft.Office.Server.UserProfiles.SocialDataStoreException. Internal error code: 0. {% endcodeblock %}

The problem seemed fairly straight-forward. The targeted site I wanted to follow couldn’t be found. However, it didn’t make sense. The instance URL I was targeting (https://chrisweldon.sharepoint.com) did in fact exist. Furthermore, I was not already following that site. So, why was this failing?

Let’s take a look at some code:

{% codeblock lang:javascript %} SP.SOD.executeOrDelayUntilScriptLoaded(userProfilesLoaded, ‘SP.UserProfiles.js’);

function userProfilesLoaded() { SP.SOD.executeFunc(‘userprofile’, ‘SP.Social.SocialFollowingManager’, followSites); }

function followSites() { var context = SP.ClientContext.get_current(); var socialManager = new SP.Social.SocialFollowingManager(context); var socialSite = new SP.Social.SocialActorInfo(); socialSite.set_contentUri(“https://chrisweldon.sharepoint.com”); socialSite.set_actorType(SP.Social.SocialActorType.site); socialManager.follow(socialSite);

context.executeQueryAsync(
    function() { alert('Sites followed!'); }, 
    function(sender, args) { alert('Error: ' + args.get_message()); });

} {% endcodeblock %}

This is fairly straight-forward. I simply setup a social actor to follow and call out to the SocialFollowingManager to attempt to follow that site. This is when I thought, perhaps this is a permissions problem? I’m trying to have the JavaScript object model write a request to the Social datastore. Perhaps it wasn’t authorized.

I changed the permissions in my AppManifest.xml from Read to Write:

Sadly, I received the same error message. However, if you read the MSDN Documentation on Developing Social Features in SharePoint 2013, there is a section in there talking about user profiles:

{% blockquote %} User Profiles (http://sharepoint/social/tenant) The permission request scope used to access all user profiles. Only the profile picture can be changed; all other user profile properties are read-only for apps for SharePoint. Apps that request rights for the User Profiles scope must be installed by a tenant administrator. {% endblockquote %}

In a nutshell, I have to grant Tenant permissions to my app to be able to have my user follow a new site. Therefore, my new permissions look like the following:

As indicated by the paragraph above, the app now needs to be installed by a tenant administrator. However, in doing so, the app now follows sites (and other content) with ease. Why the obscure error message? That I don’t know, and I hope the SharePoint team might look to address this with a more correct error message in the near future.

{% img center /images/posts/2013-04-30-sorry-only-tenant-administrators-can-add-this-app/error.png Error Message %}

The application required Tenant Read permissions as I’m accessing the User Profile through the JavaScript object model. The purpose of using the User Profile JSOM is to get the suggestions of sites and people to follow that SharePoint presents.

Now, the concept of a “Tenant” makes sense for Office 365 or SharePoint Online. As a hosting provider, there are multiple tenants you want to support in a single environment. However, for an on-premise deployment, this error message didn’t make much sense. I started poking around and came across spinning up a tenant administration site, being able to set multiple app tenants through the App Management Service cmdlets, but none of those really seemed like the right solution for my on-premise deployment. I found an MDSN Forum Question which seemed closer to the solution. That post recommends splitting the service accounts used to host the App Management and Site Settings services from the farm account. This was critical as the Farm Account is not allowed to add apps under its identity whatsoever. You will get an error message when trying to provision, and the ULS logs will indicate that an assertion failed checking that the current account was not the system account.

{% img right /images/posts/2013-04-30-sorry-only-tenant-administrators-can-add-this-app/success.png Provision %}

What did it turn out to be? I just needed to make sure my user account was directly added as a member of the Farm Administrators group. We have traditionally deployed farm administrators via an Active Directory and local (Administrators) group. However, it appears that the App Management service dislikes this approach and wants users explicitly permissioned to the Farm Administrators group. Additionally, after granting your user direct permissions, you need to issue an iisreset so those changes take effect. Then, you can provision your app successfully.

When moving this workflow into my customer’s environment, they require I specify a JIRA ticket on each commit to the repo. I had forgot to do that for the first few commits, which looked like this:

{% codeblock %} $ git log –oneline

6fa0719 Fixing the handlebars template to render the target as parent. 21ad0cf Initial commit of the SiteSuggestions app 6430b3b Adding the .gitignore files 095d92e SP-34: Creating branch to track the new suggestions app part. {% endcodeblock %}

So, when I tried to commit, I got an error indicating that the commit failed. In looking at the error, I saw the pre-commit hook error requiring that I associate the commit with a JIRA ticket. So, my first comment was easy to fix:

{% codeblock %} $ git commit –amend -m “SP-34: Fixing the handlebars template to render the target as parent.” [2013-sitesuggestions-svn 6b4bc64] SP-34: Fixing the handlebars template to render the target as parent.“ 1 file changed, 1 insertion(+), 1 deletion(-) {% endcodeblock %}

Now I just had to figure out how to modify commits 21ad0cf and 6430b3b. I tried to use git interactive rebase as per the git svn tutorial and per the git-scm book. However, in both cases, I got the following error message:

{% codeblock %} $ git rebase -i HEAD-3 fatal: Needed a single revision invalid branch HEAD-3 {% endcodeblock %}

The solution was to specify the SHA for the commit:

{% codeblock %} $ git rebase -i 6430b3b^ {% endcodeblock %}

This started the rebase process and allowed me to amend the commits for every faulted commit. Now, everything looks correct:

{% codeblock %} $ git log –oneline

6fa0719 SP-34: Fixing the handlebars template to render the target as parent. 21ad0cf SP-34: Initial commit of the SiteSuggestions app 6430b3b SP-34: Adding the .gitignore files 095d92e SP-34: Creating branch to track the new suggestions app part. {% endcodeblock %}

When I finally committed using git svn dcommit, everything went perfectly!