Wednesday, November 15, 2017

Using the MemoryCache class can save your application a great deal of time by saving the results of expensive operations such as a database query or a web service requeste. However, great care must be taken to ensure that managing the scope of the cache and key management does not lead to headaches.
The best recommendation is to let the MemoryCache class handle its own scope by using the Default cache item. 
var memoryCache = MemoryCache.Default;
The class automatically lazily loads and references a singleton cache of objects regardless of when the class is referenced in your application. This is a good thing for your application because it means the cache is always there for you and you don't have to worry about the scope of the cache or the objects using it.
Using the cache can be as simple as the example below shows.
    public class FooProvider
    {
        private readonly MemoryCache _defaultCache = MemoryCache.Default;
        private readonly FooExpensiveProvider _fooExpensiveProvider = new FooExpensiveProvider();

        public Foo GetFooItem(string name)
        {
            var fooItem = _defaultCache.Get(name) as Foo;
            if (fooItem == null)
            {
                fooItem = _fooExpensiveProvider.GetFooItem(name);
                _defaultCache.Add(name, fooItem, DateTimeOffset.UtcNow.AddHours(1));
            }

            return fooItem;
        }
    }


    public class FooExpensiveProvider
    {
        public Foo GetFooItem(string name)
        {
            return new Foo
            {
                Name = Guid.NewGuid().ToString(),
                Content = "Content"
            };
        }
    }

With this in place, we can write a simple test to see it working.
    class Program
    {
        static void Main(string[] args)
        {
            var fooProvider = new FooProvider();

            Parallel.For(0, 1000, i =>
            {
                var fooItem = fooProvider.GetFooItem("test");
            });

            Console.WriteLine("CacheMissCount = " + fooProvider.CacheMissCount);
        }
    }

Whoops! We got back more than 1 cache miss! What happened? In our case, there are multiple requests to the FooProvider's GetItem method that are running concurrently. Multiple calls entered the code at the same time, found the cache empty and performed the expensive operation then updated the cache. 
We can prevent concurrent requests from performing the expensive operation by first locking the code  around the desired area.
    public class FooProvider
    {
        private readonly MemoryCache _defaultCache = MemoryCache.Default;
        private readonly FooExpensiveProvider _fooExpensiveProvider = new FooExpensiveProvider();
        private static readonly Object _cacheLock = new object(); // must be static!

        public int CacheMissCount = 0;

        public Foo GetFooItem(string name)
        {
            var fooItem = _defaultCache.Get(name) as Foo;
            if (fooItem == null)
            {
                lock (_cacheLock)
                {
                    fooItem = _defaultCache.Get(name) as Foo;
                    if (fooItem == null)
                    {
                        fooItem = _fooExpensiveProvider.GetFooItem(name);
                        _defaultCache.Add(name, fooItem, DateTimeOffset.UtcNow.AddHours(1));
                        Interlocked.Increment(ref CacheMissCount);
                    }
                }
            }
            return fooItem;
        }
    }

Now when we run the test program, we get back the expected single cache miss. Notice also, that it is necessary to again check the cache after locking the code area, because it is possible another request ahead of the current one updated the cache between the first cache check and locking the code area.
We can also demonstrate that by using MemoryCache.Default, multiple instances of the FooProvider class still use the same singleton cache and the cache item is only missed once.
    class Program
    {
        static void Main(string[] args)
        {
            Task[] tasks = new Task[3];

            tasks[0] = Task.Run(() =>
            {
                var fooProvider = new FooProvider();
                for (int i = 0; i < 100; i++)
                {
                    var fooItem = fooProvider.GetFooItem("test");
                }
                Console.WriteLine("FooProvider.CacheMissCount = " + FooProvider.CacheMissCount);
            });

            tasks[1] = Task.Run(() =>
            {
                var fooProvider = new FooProvider();
                for (int i = 0; i < 100; i++)
                {
                    var fooItem = fooProvider.GetFooItem("test");
                }
                Console.WriteLine("FooProvider.CacheMissCount = " + FooProvider.CacheMissCount);
            });

            tasks[2] = Task.Run(() =>
            {
                var fooProvider = new FooProvider();
                for (int i = 0; i < 100; i++)
                {
                    var fooItem = fooProvider.GetFooItem("test");
                }
                Console.WriteLine("FooProvider.CacheMissCount = " + FooProvider.CacheMissCount);
            });


            Task.WaitAll(tasks);
        }
    }

Using Named Caches

It is possible to used a named cache instead of the Default. It is strong recommended that you do not do this because it requires you to carefully manage the lifetime and scope of the cache item. This could lead to situations where more than one cache is created and cache related objects are not released causing memory leaks.
Do not create MemoryCache instances unless it is required. If you create cache instances in client and Web applications, the MemoryCacheinstances should be created early in the application life cycle. You must create only the number of cache instances that will be used in your application, and store references to the cache instances in variables that can be accessed globally. For example, in ASP.NET applications, you can store the references in application state. If you create only a single cache instance in your application, use the default cache and get a reference to it from the Default property when you need to access the cache.


Caching Multiple Types of Objects

Since the recommended strategy for caching objects is to use only the Default, what if we wish to cache multiple object types? Since the cache can store any type of object, we can simply differentiate the cached items by prefixing the object type to the cache key. Notice in the example below that the only difference between this example and the previous examples is that the cache key is different - it uses a prefix to denote what kind of object we're after. Note that this technique could also be used to cache the same object types by different cache keys.
        public Foo GetFooItem(string name)
        {
            string cacheKey = string.Format("Foo.ByName.{0}", name);
            var fooItem = _defaultCache.Get(cacheKey) as Foo;
            if (fooItem == null)
            {
                lock (_cacheLock)
                {
                    fooItem = _defaultCache.Get(cacheKey) as Foo;
                    if (fooItem == null)
                    {
                        fooItem = _fooExpensiveProvider.GetFooItem(name);
                        _defaultCache.Add(cacheKey, fooItem, DateTimeOffset.UtcNow.AddHours(1));
                        Interlocked.Increment(ref CacheMissCount);
                    }
                }
            }
            return fooItem;
        }

Monday, November 6, 2017

The Good, the Bad, and the Ugly of the static keyword

I've been working in a lot of legacy code lately. One of the issues that has come up repeatedly is the misuse and abuse of the "static" keyword in C#. It would be easy for me to knee-jerk and say "static" is just evil, but that would be a disservice to a useful tool that can totally make sense in the right situations. So before I talk about the negatives, let's look at the good stuff!

The Good

Pure Methods

One of the best uses of static is the ability to create pure methods (functions). A pure function meets these requirements (totally stolen from Wikipedia):

  1. The function always evaluates the same result value given the same argument value(s). The function result value cannot depend on any hidden information or state that may change while program execution proceeds or between different executions of the program, nor can it depend on any external input from I/O devices.
  2. Evaluation of the result does not cause any semantically observable side effect or output, such as mutation of mutable objects or output to I/O devices.

Pure functions or methods have several good benefits, including making the program easier to read and reason about, as well as preventing unwanted side-effects. Another tangible in .NET is that static methods can potentially be faster since the run time does not require an object instance to be referenced to run the method. However, I would caution against using static for performance gains unless you can actually measure the difference between using it and not using it. It's too easy to prematurely optimize code that doesn't need it at the cost of readability, or worse, creating something that commits some of the sins I'll list later on. 


Extension Methods

Another great use of static methods is extension methods. The ability to write fluent statements that alter an object, even when you don't own the code for that object, is an incredibly useful feature. While at first glance it does seem to violate the idea of a pure function, extension methods use the "this" keyword to explicitly indicate that a side effect is intended and desired. It's a contract between the caller and the method to do something to the input, so it's all good.

The Bad

Impure Functions with Side Effects

One of the easiest ways to misuse static is to have a function that alters its input arguments. One of the biggest code smells for me is seeing a method that starts with "static void". It means the author is intentionally creating a method full of side effects on objects that the method doesn't own! This is a clear violation of SOLID in terms of encapsulation and the single responsibility principle. It usually indicates some bit of logic that needs to belong to the object itself or some kind of factory or provider class. The best way to address this kind of flaw is to move the logic to a public method of the targeted class or create an extension method. If the logic is creating a series or collection of objects, consider a factory class instead.

The Ugly

Singletons and Static Properties

OK, there may be a few times in your programming career where you find that a singleton of something makes sense. The really important thing is how you implement it, and using the static keyword should not be it. Using static to create a singleton is literally just creating a giant global variable for the scope of the entire application and the programmer using the class can do squat about it. Instead you should use a IoC framework to control the scope of the object as necessary. The application should be able to choose how many of something it needs, and it shouldn't be the decision of the class itself. There are other downsides, not the least of which is that this often makes the class not thread safe. In addition, it is often difficult to unit test singleton classes because unit tests run concurrently may yield unpredictable results (i.e. a result of not being thread safe).

Static properties are another land mine of this idea of using a keyword to create global variables in a class. Not only are they not thread safe, they can result in race conditions where shared state is involved.

Static Dependencies

Probably the worst abuse in static methods (in my opinion) is the inclusion of other static classes and methods being referenced in the static method. This is a clear violation of SOLID and literally hard-wires external dependencies to your class & method, not to mention making them potential global variables. This is also most likely not thread safe (unless you happen to reference only pure functions), so in general is likely to cause you all sorts of headaches at design time and run time. 

Conclusion

There are cases where the static method could genuinely help your code. But in most cases, I see so much confusion and abuse of the keyword that I recommend avoiding it unless you can prove to yourself that a pure function genuinely helps the readability and performance of your code. 




Friday, April 11, 2014

Why the proposed null-propogating operator in C# may be bad for the .NET community.

As one of many developers who is excited about the new Open-Source "Roslyn" compiler in .NET, I can't help but get excited about new language features for C# that could be included in the near future. However, there is one feature that I have mixed feelings about. That proposed feature is the "null propogation operator". It is meant to address issues where people find elaborate ways of working around the issue of "deep null checking". There has also been a lot of discussion about the "Law of Demeter" in various places in an attempt to address this issue. While it is the most requested feature for the language it strikes me as one of those things that is added to make things more convenient for programmers that may instead be an indication of code smell. In other words, if you need this feature, you may be "doing it wrong".

Pardon me while I tell a story to lay the foundation of my argument. The story is true, although I may be embellishing some of the details. Back in the mid-90's I was a humble C.S. undergrad new to the ways of programming. In my junior year, I had a class called "Object Oriented Programming". I had already learned C and C++ and was not unfamiliar with classes and objects. However, the class was focused on using the Smalltalk language, where everything is an object. What better way to learn OOP correctly? The professor was a Hagrid-like man, big and burly with a long beard. Our first lecture he gave a powerful demonstration of what OOP should be about. He got a volunteer to stand with him in the front of the class. He then demanded of the student, "Give me a dollar!" The student eyed him carefully, then finally pulled out his wallet and gave him a dollar. The professor beamed and said, "How would you feel if instead I had just reached into your back pocket, pulled out your wallet and took out a dollar?" "Not good." "That is the difference between OOP and the kind of programming you are probably used to. In this class, we will learn to stop picking people's pockets."

The point of me telling this story is that many programmers don't understand what "encapsulation" means. They treat objects as a convenient data store and nothing more. Almost all properties of an object are public and they reach in and pick the pockets of the object without any regard for encapsulation.

So when I see something like this...

 if (cake != null && cake.frosting != null && cake.frosting.berries != null)  

...I can't help but cringe. The programmer is peeking into the object's pockets to see what it has or doesn't have and what can be safely used to do something important. In this case, the programmer is using the properties of the cake to determine what kind of cake it is instead of asking, "What kind of cake are you?"

Let me say first that I understand and appreciate sanity clauses for null objects, especially when using inversion of control techniques like dependency injection. However, it is my opinion that you should never have a null object as a valid return value unless you specifically use a nullable type. If your source for an object gives you null instead of an object and you aren't expecting that, you should simply let things fail ("fail early, fail often") and find out why things failed. The biggest source of code smell here is that we are forcing ourselves to check for null when null should be exceptional outcome and not an expected outcome. Programmers who do OOP this way are willingly paying a null-check penalty every time they need to inspect a property. But that doesn't have to do be the case.

Let us say, for argument's sake, that we are trying to display the contents of a cake on a web page. Wouldn't you like your view to be able to assume that the cake existed, for starters? The view should be "dumb" in that it shouldn't require any special knowledge of cake objects in order to display a description of the cake. Should we have to check for the existence of berries (and frosting, and the cake itself!) to see if we should display berries and what kind of berries we should display? Or can we design a class hierarchy that doesn't require null checking to be used easily? In other words, can we design objects that tell us what they are without having to put our greasy mitts in their pockets?

What if a Cake object consisted of a collection of Ingredient objects? Each Ingredient object could have an IngredientType that contained an enum and/or string description of the ingredient? We could further fill out this tree of objects to included a Topping class and ultimately a BerryTopping class. By the time we're done our new code for displaying berry toppings now looks something like this:

 var berryToppings = cake.ingredients.OfType<BerryTopping>();    
 foreach (var bt in berryToppings)  
 {  
    // display the berry toppings  
 }  

No null checking needed! We simply ask the cake, "Do you have berry toppings?" And it replies with a collection of objects which we can enumerate and display. Now if some property in an ingredient is unexpectedly null, we should stop and examine the code that allowed such a condition to exist rather than adding another null check.

What is my point? It is that if you take the time to properly design your objects such that null references are truly unexpected, then you don't need deep null checking or a special language feature to make deep null checking convenient. I worry that if the Roslyn team moves forward with this feature, they will just be enabling bad OOP practices. In other words, they'll just be enabling.NET programmers to be better pick-pockets!

Edit: Many thanks to sharp redditors for improving my example!


Thursday, February 6, 2014

Web Developers are insane

It's often said that the definition of insanity is doing the same thing over and over again while expecting different results.

As a web developer dealing with multiple browser compatibility, I commit acts of insanity every day.  I write code that I expect to have the same outcome in all browsers and it simply doesn't happen. Quite often when dealing with older versions of Internet Explorer. I really want to be the first to cheer Microsoft for doing the right things lately in terms of embracing open standards, but they have a lingering legacy of incompatibility that makes me curse daily. Happily, I don't have to deal with anything as arcane as IE6 or even IE7. But working in the healthcare industry, the large majority of our users are on still on IE8 and XP. It seems that almost every time we add a feature or fix some bug, it breaks the functionality of something else in IE8. I've spent the clear majority of my effort on the current "sprint" dealing with IE8 issues. And the rest have been quirks of other browser and sameness.

All this makes me hope that the death knell of Windows XP will also spell the doom of IE8 as well.

So I guess not all web developers are insane. Just us devs who still have to work with IE8.

Wednesday, April 17, 2013

Today I ran into a problematic bug that took me awhile to identify the real problem.

I have a set of extension methods for automatic retires of Sql commands.

The extensions look like this:

Code:
namespace SqlExtensionsTest
{
    public static class SqlExtensions
    {
        public static T Retry<T>(this Func<T> command, int retryCount, ILog logger)
        {
            int count = 0;
            while (count <= retryCount)
            {
                count++;
                try
                {
                    return command();
                }
                catch (Exception ex)
                {                    
                    logger.WarnFormat("Exception in Retry operation block: retry #{0} : {1} : {2} : {3}", count, ex.Message, ex.StackTrace, ex.InnerException.StackTrace);
                    if (count == retryCount)
                    {
                        throw ex;
                    }
                }
            }
            return default(T); // this statement will never be reached, yet must be here for the method to compile
        }

        public static void RetryOpen(this SqlConnection connection, int retryCount, ILog logger)
        {
            Func<bool> func = () => { connection.Open(); return true; };
            func.Retry(retryCount, logger);
        }

        public static object RetryExecuteScalar(this SqlCommand command, int retryCount, ILog logger)
        {
            Func<object> func = () => command.ExecuteScalar();
            return func.Retry(retryCount, logger);
        }

        public static SqlDataReader RetryExecuteReader(this SqlCommand command, int retryCount, ILog logger)
        {
            Func<SqlDataReader> func = () => command.ExecuteReader();
            return func.Retry(retryCount, logger);
        }

        public static int RetryExecuteNonQuery(this SqlCommand command, int retryCount, ILog logger)
        {
            Func<int> func = () => command.ExecuteNonQuery();
            return func.Retry(retryCount, logger);
        }        
    }
}


Do you see the problem?

No?

Answer: In some cases the exception may not have an inner exception object! Ugh. It's always better to simply let the implicit ToString method do it's job. We can rewrite the log entry like so:


logger.WarnFormat("Exception in Retry operation block: retry #{0} : {1}", count, ex);
                    if (count == retryCount)
                        throw;

Not only does that work all the time, it's easier to read as well. As a bonus, we rethrow the original exception withou the "ex" (the original has the side effect of losing the original stack info).



Tuesday, November 29, 2011

How to interview well for a programmer position - Part 1

Our team is currently trying to add new programmers. We've been on the lookout primarily for seasoned programmers with good .NET skills, but we've recently broadened our search to entry level candidates simply because there seems to be more demand than qualified candidates these days. This is good news for those folks looking for work or looking to move up, but it makes interviewing candidates a potentially disheartening affair simply because candidates come unprepared.

If you are indeed serious about being a hire-able programmer, there are some things we look for, and these should be true no matter where you end up working. The first and most important quality we are looking for is that you are passionate about programming. What are you doing on a weekly basis to improve your skills? What new languages or technologies are you learning and exploring? What blogs do you read? What podcasts do you listen to? How many tech books have you read through this month? Are you involved in any programmer groups, local meetings, or code camps? Do you have a side projects you're working on? Are you ready to show off some sample code? Work on any code katas lately? If I ask you these questions and you come up with next to nothing, I am going to be tempted to ask you to leave the interview early. Anything I ask you after this may reveal your ability to do work, but it doesn't mean you're the kind of person who enjoys programming and actually wants to show up to work every day. If you're not doing any of these things, please start or find another line of work!

When it comes to the interview itself, be prepared to actually write some code! That seems fairly obvious, but I swear 90% of the candidates have a look of panic when I hand them an Expo marker and then ask them to sketch out a simple method. Remarkably, most candidates struggle with such a simple exercise which entails nothing more than what should already be second nature to you if you're a bona fide programmer. On one job interview I went on, the hiring manager sat me in front of a computer and asked me to program a simple web page for a blog comment entry system. He dictated the specs as I wrote it and was so impressed that I could actually do it that he was ready to hire me on the spot. Too often, I see people struggle with the simplest algorithm. Be ready to code "on your feet", and that just means more practice on your desired platform until Google is a last resort, not your first instinct. I highly recommend coding katas  as a practice tool.

Look, I'll even give you a freebie. I typically start with a simple problem such as "given a string that contains your first name, write a method that returns a string with the spelling reversed". Sounds easy, right? It is, but if you take all 30 minutes of our interview time to solve it, you won't be working with me. Something that trivial should take you 2 or 3 minutes, and you should probably be able to solve it in at least 2 different ways. (If you're a .NET programmer, you could use LINQ to do it in 2 lines.)

The same is true for databases. If your previous jobs involved writing SQL queries and stored procedures, you can better believe that I am going to ask you to write up a few simple queries to prove that you can actually do it without Googling. Be ready to know how to write a simple JOIN, and for gosh sakes know the difference between an inner join and an outer join.

The bottom line is that you need to be ready to prove to the hiring manager that you can do the job without a lot of help. The best way you can do that is practice your programming skills in your target platform so that when people ask for something easy, you don't hesitate and you simply do it.

Monday, November 14, 2011

To length or not to length - that is the question

I needed to find a way to determine if a Silverlight app had been loaded in a div of an iframe. I wasted some time  trying to set a boolean flag and tracking events trying to get it set correctly. jQuery to the rescue! The consensus seems to be that this is the best way to check for the existence of a tag:

if ( $('#targetdiv').length > 0 )

or if you want to be tricky

if ( $('#targetdiv').length )

Of course, my target div was in an iframe so I used this:


if ( $('#ResultsFrame').contents().find('#silverlightControlHost').length > 0 )