SOLID Architecture and the Decorator Pattern

My last post was on the Single Responsibility Principle and the repository pattern. I received a comment back that my implementation would be cleaner if I had used the decorator pattern. This would allow for easy composition of data flow without creating additional dependencies on particular data stores.

This really set my mind a twitter. Could I do that? Why hadn’t I done that already? With this in mind I set out to look deeper into the decorator.

What is the decorator pattern?

The decorator pattern is a simple way of enabling a chain of behaviour that is determined by composition, not by inheritance or configuration inside an object.

Let’s look at building a cookie without decorators.

public class Cookie
{
	public bool HasChocolateChips { get; set; }
	public bool ContainsNuts { get; set; }

	public Cookie(bool hasChocolateChips, bool containsNuts)
	{
		HasChocolateChips = hasChocolateChips;
		ContainsNuts = containsNuts;
	}

	public decimal GetPrice()
	{
		return 1.00 + (HasChocolateChips ? 0.25 : 0.00)
			+ (ContainsNuts ? 0.50 : 0.00);
	}
}

Simple enough. But as we add more and more options to the cookie, the code gets uglier and uglier. Eventually we need to do something because this doesn’t scale.

Enter the decorator pattern.

public interface ICookie
{
	decimal GetPrice();
}

public class Cookie : ICookie
{
	public Cookie()
	{ }

	public decimal GetPrice()
	{
		return 1.00;
	}
}

public class ChocolateChipDecorator : ICookie
{
	private ICookie _cookie;

	public class ChocolateChipDecorator(ICookie cookie)
	{
		_cookie = cookie;
	}

	public decimal GetPrice()
	{
		return _cookie.GetPrice() + 0.25;
	}
}

public class NutsDecorator : ICookie
{
	private ICookie _cookie;

	public class NutsDecorator(ICookie cookie)
	{
		_cookie = cookie;
	}

	public decimal GetPrice()
	{
		return _cookie.GetPrice() + 0.50;
	}
}

Wow. That’s a lot more code. It’s also gone from 1 class to 3 classes and an interface. It sounds like a maintenance nightmare, but if we are a cookie manufacturer we might have 50 different options that can be composed in many different ways so it might end up being a more scalable solution. Let’s pretend it is.

How do we make different cookies?

new Cookie();
new NutsDecorator(new Cookie());
new NutsDecorator(new ChocolateChipDecorator(new Cookie()));
new ChocolateChipDecorator(new NutsDecorator(new Cookie()));

Yeah, I just threw up in my mouth a little bit.

In fact, if we had 50 of these we would need some sort of builder class in order to handle this, because otherwise it’s just too gross.

But what else can we do? What if we had a decorator that at its base made the cookie twice as expensive?

public class OrganicFlourDecorator : ICookie
{
	private ICookie _cookie;

	public class OrganicFlourDecorator(ICookie cookie)
	{
		_cookie = cookie;
	}

	public decimal GetPrice()
	{
		return _cookie.GetPrice() * 2;
	}
}

Hmmmmmm. Something isn’t right there.

new OrganicFlourDecorator(new ChocolateChipDecorator(new Cookie())).GetPrice();
new ChocolateChipDecorator(new OrganicFlourDecorator(new Cookie())).GetPrice();

The contents of each cookie are exactly the same, but the cost of the first is $2.50. The cost of the second is $2.25. Who likes bugs? Who likes the kinds of bugs that take hours to debug and end up being stupid?

Of course, this is a bit of a straw man argument. Clearly an organic flour cookie should be a different type of base cookie. But what about a different size of cookie? Wouldn’t that logically be a multiplier since you need a different amount of mixins?

Another problem here comes with this possibility:

Cookie cookie = null;
new NutsDecorator(cookie).GetPrice();

This is a null reference exception waiting to happen unless you start polluting all of your potential method chains with null checks. This isn’t going to be caught at compile time either, meaning your users will be the first ones to experience the joy of this bug.

This whole thing is a little silly though. I mean, these are just simple examples. If it fails for simple examples that I created with failure and silliness in mind, it should reasonably work for implementing the repository pattern if I do it in a smart way, right?

public interface IArticleRepository
{
	List<Article> GetArticles();
	void SaveArticles(List<Article> articles);
}

public abstract class RepositoryBase : IArticleRepository
{
	private IArticleRepository _baseRepository;

	public RepositoryBase(IArticleRepository articleRepository)
	{
		_baseRepository = articleRepostory;
	}

	public List<Article> GetArticles()
	{
		List<Article> articles = null;

		if (_baseRepository != null)
		{
			articles == _baseRepository.GetArticles();
		}

		return articles ?? LoadArticles();
	}

	protected abstract List<Article> LoadArticles(); 
}

public class SqlRepository : RepositoryBase
{
	public SqlRepository(IArticleRepository baseRepository)
		: base(baseRepository)
	{ }

	protected List<Article> LoadArticles()
	{
		// Load em from SQL.
	}
}

public class CacheRepository : RepositoryBase
{
	public CacheRepository(IArticleRepository baseRepository)
		: base(baseRepository)
	{ }

	protected List<Article> LoadArticles()
	{
		// Load em from cache.
	}
}

Now you can have a nice easy chain:

IArticleRepository repo = new SqlRepository(new CacheRepository(null));

It will check the cache for articles and if it doesn’t find them, will load them from the SQL database. We’ve even made an abstract base so each implementation doesn’t need to check for null on a base repository. It seems fairly light and easy to understand.

But this only works if you don’t have expiry on your cache. What if the objects in your cache expiry, and you need to reload them from your database the next time they are requested? Well, LoadArticles() in the SqlRepository needs to account for this. But because we only want them saved back in a CacheRepository and not necessarily a MongoRepository we now need to check to see what type IArticleRepository truly is in order to know how we are going to handle it.

Longer chains get even worse.

Long story short, I really looked at the decorator pattern as a possibility. I read that comment and like a lot of developers would do, I thought, “What if I’m totally wrong on this?” I wasn’t too familiar with the decorator pattern so I read up on it. I did not look for reasons it has problems.

In fact, I imagine that most of these conclusions are well known shortfalls with the pattern that people know well. If not I hope I’ve been able to give you something to think about.

I’m more concerned with making my code readable and maintainable than implementing patterns.

SOLID Architecture and the Repository Pattern

The repository pattern is super common.

It’s a layer in your code that exists between your business code and your database. It helps isolate code that interfaces with your database in a single place. Without it, you end up with data access code sprinkled throughout your codebase.

What happens if you don’t use the repository pattern? In the case that you update a table or stored procedure in the database, you need to find every spot in your code that relies on it and update all of them.

With the repository pattern you only need to update a single class and the rest of your code is none the wiser.

Yet I find a lot of confusion in platform architecture with the repository pattern. When I see implementations I always end up seeing things like validation, business logic, and a mix of things like caching in with bare metal data access. This might sound like a good thing, except a generally see these things all jammed into a single class.

Developers who are familiar with the Single Responsibility Principle seem to forget all about it when it comes to implementing the repository pattern. All of a sudden a “data access layer” (DAL) is born, but it’s not really a layer. It’s just a single class.

When I talk to developers about this, or read blog posts about DAL creation, it seems to be a very common case that people forget about the SRP, or believe that the DAL should not follow the SOLID principles. Many times the reason ends up being that they believe their code is too simple to have multiple components in the DAL.

But then something changes, they want caching, validation, auditing, or logging. These can be great things and should be desired, but they are added directly into the unified DAL class or into Helper classes without considering the overall design of the “layer,” which is now really more of a god class.

What is the Single Responsibility Principle? How can it help us design a better data access layer?

The Single Responsibility Principle states that each thing should have a single reason for change. This helps us isolate changes, and limit the scope of a class to a single level of abstraction. In the end, it’s largely about readability and maintainability.

What can possibly go wrong?

Let’s start with a single DAL method. It was built with SQLClient for bare metal database access and runs off of stored procedures.

public List<BlogArticle> GetArticles()
{
    using (var connection = new SqlConnection(connectionString))
    {
        var command = new SqlCommand("dbo.BlogArticles_Get", connection);
        command.CommandType = CommandType.StoredProcedure;
        connection.Open();

        var articles = new List<BlogArticle>();

        using (SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                articles.Add(new BlogArticle(reader[title], reader[publishDate]));
            }
        }

        return articles;
    }
}

This looks good. Nice and easy and only one reason to change — because there was a change to the stored procedure. But, at some point someone realizes that there’s a performance issue developing. This is called a lot due to some of the front end design and how users interact with our service. A simple solution is simple to make fewer calls to the database and add in a server level cache. We only need caching in this one spot right now, so we won’t bother adding caching everywhere. As a result, this is what the method becomes:

public List<BlogArticle> GetArticles()
{
    ObjectCache cache = MemoryCache.Default;
    List<BlogArticles> articles = cache["articles"] as List<BlogArticle>;

    if (articles != null)
    {
        return articles;
    }

    using (var connection = new SqlConnection(connectionString))
    {
        var command = new SqlCommand("dbo.BlogArticles_Get", connection);
        command.CommandType = CommandType.StoredProcedure;
        connection.Open();

        using (SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                articles.Add(new BlogArticle(reader[title], reader[publishDate]));
            }
        }
    }

    var policy = new CacheItemPolicy();
    policy.AbsoluteExpiration = DateTimeOffset.Now.AddSeconds(5.0);

    cache.Set("articles", articles, policy);
    return articles;
}

This code gets added to production and your performance improves as expected. Next, you are alerted that the database is being accessed and blog articles are being pulled by people who don’t have permissions. A discussion is had about permissions and you can’t think of a case where you would ever want to get the blog articles unless the current user has permission to do so. That code then gets added to the DAL.

public List<BlogArticle> GetArticles()
{
    if (!Request.User.Roles.Contains("blogReader"))
    {
        throw new Exception("User lacks permissions to view articles.");
    }

    ObjectCache cache = MemoryCache.Default;
    List<BlogArticles> articles = cache["articles"] as List<BlogArticle>;

    if (articles != null)
    {
        return articles;
    }

    using (var connection = new SqlConnection(connectionString))
    {
        var command = new SqlCommand("dbo.BlogArticles_Get", connection);
        command.CommandType = CommandType.StoredProcedure;
        connection.Open();

        using (SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                articles.Add(new BlogArticle(reader[title], reader[publishDate]));
            }
        }
    }

    var policy = new CacheItemPolicy();
    policy.AbsoluteExpiration = DateTimeOffset.Now.AddSeconds(5.0);

    cache.Set("articles", articles, policy);
    return articles;
}

This code now has a lot of reasons to change. If the caching changes, the database changes, or the permissions that we want to check change we will need to update this class. And of course you’re going to set an example for other people on your team who will possibly imitate the functionality of your method when adding new ones.

What would I propose in its place?

Separation of concerns. What we have is going to become difficult to grow and maintain. The first change is to move the bare metal database access to its own class.

public List<BlogArticle> GetArticles()
{
    if (!Request.User.Roles.Contains("blogReader"))
    {
        throw new Exception("User lacks permissions to view articles.");
    }

    ObjectCache cache = MemoryCache.Default;
    List<BlogArticles> articles = cache["articles"] as List<BlogArticle>;

    if (articles != null)
    {
        return articles;
    }

    articles = _dataContext.GetArticles();

    var policy = new CacheItemPolicy();
    policy.AbsoluteExpiration = DateTimeOffset.Now.AddSeconds(5.0);

    cache.Set("articles", articles, policy);
    return articles;
}

public List<BlogArticle> GetArticles()
{
    using (var connection = new SqlConnection(connectionString))
    {
        var command = new SqlCommand("dbo.BlogArticles_Get", connection);
        command.CommandType = CommandType.StoredProcedure;
        connection.Open();

        var articles = new List<BlogArticle>();

        using (SqlDataReader reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                articles.Add(new BlogArticle(reader[title], reader[publishDate]));
            }
        }

        return articles;
    }
}

Now database changes have been isolated to a single class with a single reason to change. Next is to move the caching over to its own class.

public List<BlogArticle> GetArticles()
{
    if (!Request.User.Roles.Contains("blogReader"))
    {
        throw new Exception("User lacks permissions to view articles.");
    }

    List<BlogArticles> articles = _cacheContext.GetArticles();

    if (articles == null)
    {
        articles = _dataContext.GetArticles();
        _cacheContext.CacheArticles(articles);
    }

    return articles;
}

public List<BlogArticle> GetArticles()
{
    return _cache["articles"] as List<BlogArticle>;
}

public void CacheArticles(List<BlogArticle> articles)
{
    var policy = new CacheItemPolicy();
    policy.AbsoluteExpiration = DateTimeOffset.Now.AddSeconds(5.0);

    _cache.Set("articles", articles, policy);
}

Cache access has now been isolated to a single class. It changes when the details of caching change. Next, move out the permissions check. Permissions should be checked in the layer where you actually care about a user object. Putting them in your repository is going to limit the use of your repository to times when you have a contextual user. This might not seem limiting now, but you might find that it requires significant rework later.

public List<BlogArticle> GetArticles()
{
    List<BlogArticles> articles = _cacheContext.GetArticles();

    if (articles == null)
    {
        articles = _dataContext.GetArticles();
        _cacheContext.CacheArticles(articles);
    }

    return articles;
}

What reason does the repository have to change now? It changes if the workflow of data access changes. The method operates on only a single layer of abstraction and doesn’t need to change when bare metal data access changes to caching or to the database.

We end up with three classes instead of one, but a significant readability and maintainability improvement.

What reasons does this code have to change? The repository method only needs to change if the flow of data access changes — as in, logging or auditing is added.

The code in the other classes need to change if caching or the stored procedures change, but that code is now isolated away from the data access workflow and no longer impacts it directly.

As an added bonus, it’s likely that others will see how the data access has been done on this method, and when caching needs to be added elsewhere they will follow the existing pattern. Having worked using both styles of data access, I can verify that working with a repository built on SOLID adds a lot more velocity to future feature addition than one without proper separation.

Developers Are Not Second Class Citizens

I had an interesting conversation with a coworker today where he claimed that developers are really second class citizens in our business.

I don’t know if I would agree with this. Let me provide a little bit of setup for this one.

Where I work, the company has everyone on a single floor in a building. The floor layout is such that everyone who works in engineering and IT is on one half of the floor. The other half of the floor is composed of sales, marketing, services, project management, etc. The elevators, lobby, and a string of offices in the middle of the floor keep these sections separated.

You would think this is a benefit since developers often state that they want to be isolated from these groups in order to have a quiet environment. If you look at this in action though, it’s actually helped to develop a culture that has a bit of “us vs them,” from the engineering point of view. (I can’t comment on the view from the “business” side.) You will hear developers talking about “that side” almost as if it were an entirely different piece of the company focused on an entirely different set of goals.

Teams are focused on the same goal though — make our company the best in our industry and continue to work hard until that goal is met.

The point raised is that if you want to know who the company truly values, compare salaries. They believe that the salaries on the other side are higher on average than the salaries on the engineering side. This may or may not be true, I honestly have no idea. I also don’t take my value from my salary versus the salaries of those around me. More on this later.

I can advocate that developers should not be treated in a manner that makes them feel like second class citizens. I think most people would be able to agree with this.

The fact is if you were to have no one working in your engineering teams then you would have no product. You can have great marketing that drives sales, and a great sales team that drives growth, but without developers you’d have no product to sell. You would quickly find that your product is being outclassed by companies that have evolving products. You’d also end up having a hard time scaling your product to an increasing customer base with no development team to analyze where scaling is needed and how to optimize the code for scale.

I read an article last week titled “Why Women Shouldn’t Code” by Francine Hardaway. This article makes a lot of claims that I don’t agree with:

  1. Developers don’t get to make important decisions.
  2. Developers are told by the important people what to do.
  3. Development is an entry level position.
  4. Developers will be a glut in a few years.
  5. Developers don’t often rise to CEO.

This post isn’t about this article, but it seems to make the point that developers are indeed second class citizens.

It is simply not my experience that developers don’t get to make important decisions and are just told by important people what to do. If you do not believe that making decisions that dictate the performance of your product are not important decisions then you are wrong. The performance of your product is almost certainly important to your customers. It is rare to have a product focused company where your product is so highly desired that it doesn’t need to work well.

It is true that developers don’t often rise to CEO. Certainly not within a company, although they may rise to CTO or CIO, it is often the COO that ends up being groomed for CEO succession. However, you will find that developers become CEO when they start their own businesses — which they have the ability to do since they can quite literally create value from nothing.

It is also true that development is an entry level position. But it is also an intermediate and senior position. Development is a career path. It is not a single job. Like other career paths, as you become senior in your function your responsibilities increase and so does your compensation. One area where the business side of the house differs from the development side is the speed at which people rise to senior positions. Someone might look at general salaries of both groups and decide that the business professionals are more highly valued, but the amount of time people have put into their careers is not necessarily the same. Personally, I switched career paths into development and my salary dropped like a rock. That should be expected since I came in at an entry level instead of with the years of experience I had on the business side.

Lastly, I don’t believe that there will be a glut of developers but how I wish that were true. If there is a glut of developers then I fully expect half of them to be unemployable. Anecdotally, my CEO told me once that if he posts a position for something that is not a developer position he gets 70 applicants in a week. When a development position is posted we get almost no applicants. The market is just that tight in our area. On top of that, the majority of developers we interview don’t make the cut. We simply can’t hire someone who doesn’t make the grade since a bad developer has the ability to infect the product at its core and cause significant damage to the brand. It also becomes much more important to keep the developers that you have. Losing even a single developer can hurt since it can take a significant investment to backfill even a single position.

I definitely do not believe that developers should be treated as second class citizens, but on top of that I don’t think that we’re treating them as second class citizens where I work now either.

How the Heck Did Spotify Convert Me to a Premium Account?!

I’ve used a number of services (both legitimate and illegitimate) over the years to listen to music. Just recently I was an avid user of Grooveshark, whose legitimacy is in question. On September 30th, Spotify became available in Canada. It has approximately 40 million customers, and around 10 million of them are paying Premium customers. I’m now one of them, but I certainly never intended to be.

Spotify Logo

Around the time Spotify was preparing to launch, some of my coworkers were trying to convince me to sign up. I had a lot of music set up on Grooveshark. The service was free, and I really didn’t want to set up all of my music on yet another service. Regardless, out of curiosity — hey, it’s free after all — I signed up and give it a try.

Honestly, I really didn’t want to like it, yet the interface on both Windows and iPad was excellent. It responded in just about every way that I wanted.

Compared to Grooveshark it was night and day. On Grooveshark I noticed that the interface was clunky and it was painful to organize my music. Sometimes music just simply wouldn’t load. My music would occasionally disappear — I assume this happened in response to takedown requests. Music was of varying quality, which makes sense since the source of music is the hard drives of other users.

On Spotify, generating playlists was easy, and my music was automatically organized by song, album, and artist — in fact the only thing not there is genre and I’ve had to create playlists for that specifically. They also have playlists available daily in a number of styles that allow me to discover music I wouldn’t normally play. Best of all, the music I find is high quality and always plays.

It wasn’t long before I stopped using Grooveshark entirely.

But Spotify gives its users all of this for free.

What are the differences between free and premium? Premium gives the user access to an extreme quality setting. On my Android phone premium allows me to pick the song I want to listen to instead of a shuffle play. Free has ads every 15 minutes or so. Premium allows me to download songs for offline play.

Premium is $10 / month. I’m confident that over time purchasing my music on a store like iTunes would actuallly by cheaper than sustained payment to Spotify.

One of the things that pushes me to Spotify over iTunes is that Spotify is platform agnostic. Regardless of whether I’m using iOS, Android, or Windows I still have access to all of my music. My music also exists in the cloud. While there is an option for an offline sync, it’s not required. If I get a new device, all of my music is immediately accessible, unlike with thousands of music files stored on my media server at home.

Again though, all of that is free.

How did Spotify convert me to a premium account?

The experience is simply worth the money.

It might all be free, but I am willing to pay for quality and I’m willing to use my wallet to show what I like. I have changed my settings to increase the music quality, and the lack of ads is nice. I still don’t make use of offline play, but I don’t need the feature in order for them to get my support.

If you are making a service, make sure the quality is there. If I like the service and the quality is not lacking, I’ll open my wallet for your service as well.

On Hiring and Trivia Questions

I’ve talked about FizzBuzz. I’ve talked about years of experience. But there’s another topic to cover which hits almost every developer interviewing for a position: trivia questions.

Everyone's favourite game of pointless knowledge.

I need to define what I mean by trivia question. A trivia question is a question that has a definitive answer that can easily be searched on Google. These include things like, “What is a singleton?” And, “What’s the difference between a thread and a process?”

Don’t get me wrong, these are questions where experienced developers should know the answer. But, anyone can learn and memorize the answers to these questions. More than that, if someone has the capacity for learning — as all developers should — then these are things people should be able to learn and grasp in a day.

I went to an interview ten years ago for a junior developer position. I thought the interview went pretty well. The interviewers were impressed with my experience from some projects I’d been involved in, but then they asked me to list off design patterns. I thought it was an odd question — I’d never heard of design patterns. Surely, they continued, I must have at least heard of a singleton. Nope.

The thing is, I was using various design patterns. I just didn’t know the terminology. A coworker of mine today told me a very similar story. He had brought in examples of code he’d written, but he didn’t know what a singleton was, so they weren’t interested. He told me that his code actually contained a singleton, but they wouldn’t give it the time of day.

Another coworker told me that during an interview he was asked to give examples of .NET interfaces. I can’t imagine many people who work in .NET would have trouble with this question, but I also can’t see any value in it. It is so easy to learn the names of interfaces that it surely doesn’t tell you whether a candidate can actually write any code.

What’s the number one goal of an interview when you want to hire a developer? Figuring out if the candidate can actually write code that you want to maintain. If the candidate can’t write any code then there’s no point in even going further. Trivia questions do not tell you that a candidate can code, they don’t really tell you anything at all. They do give candidates good stories to tell in the future that might discourage others from applying for positions in your company.

Here’s what I would suggest: when there’s an area you really want the candidate to know about, like design patterns or interfaces, ask the question. If the candidate tells you that they don’t know but they can learn, great! Ask them to prove it by taking a week to code a small demo application that shows off the point in question. Then have another interviewer where they can teach you about the topic.

This shows that the candidate has passion, initiative, and the drive and capacity to learn new things. It will also show you that the developer can code — or show you that they can’t. The reason you ask them to teach you about the topic is so that you know they didn’t just farm out the work or copy it from Stack Overflow without actually learning.

When I’m being interviewed, this is a key tool now. If I don’t understand something or feel I got something incorrect after the interview is done, I will correct myself and write some code to demonstrate. Then I email it to the interviewer. It can only help.

(And for those who are curious, I have learned about design patterns since that interview.)

Using exceptions to break command query separation

I had a conversation with my eight year old daughter this morning about using exceptions as messaging passing.

Me: I hate it when people use exceptions as message passing.
Her: What does that mean?
Me: When you ask someone if something is ok, there are exactly two answers. What are they?
Her: Yes, and no?
Me: That’s right. Using exceptions as message passing would be when instead you either answer nothing, or scream your head off that something has gone terribly wrong.
Her: That’s dumb.

She gets it. My eight year old was able to determine that a yes or no question is not exceptional if the answer is yes or no. Why are there professional developers out there who aren’t able to determine this?

I am tired of looking at code written by professional developers that looks like this:

public void AuthenticateUser(string username, string password)
{
    string actualPassword = Passwords.ContainsKey(username) ? Passwords[username] : null;

    if (password == null || actualPassword != password)
    {
        throw new ArgumentException("Password doesn't match!");
    }
}

What is this? When you are asking if a password is valid, or if anything is valid, there are exactly two answers that are expected: yes and no.

Expected answers are not exceptional.

This method should have read like this:

public bool IsUserAuthentic(string username, string password)
{
    string actualPassword = Passwords.ContainsKey(username) ? Passwords[username] : null;
    return password != null && password == actualPassword;
}

Now it returns either a yes or a no. The user is either authentic or they aren’t.

Exceptions are great, but they should be saved for situations that you really didn’t see coming or aren’t a valid answer to the question being asked. You want a value from the database, but the database is offline? That could be an exception. You want to access a file on a disk, but file access is locked? That’s an exception. You are dividing by zero? That could be an exception.

Here’s another example:

public void ValidateDateTime(object value)
{
    DateTime parsedValue = value as DateTime;
    
    if (parsedValue == null)
    {
        throw new ArgumentException("value wasn't a DateTime!");
    }
}

This method checks to see if a value is a DateTime. If it is, nothing happens, if it isn’t an exception is thrown.

Here’s the corrected version:

public bool IsValidDateTime(object value)
{
    return value is DateTime;
}

The corrected version is even smaller than the version that throws an exception! It should be easier to write.

Part of the problem here is that, this breaks Command Query Separation (CQS). If a method has a void return, then it should be a command. It should have a side effect. These methods do not. They are queries, they just want to know if a user is authentic, or if a value is a valid type. Since they are queries they should have return types, which is why the corrected versions return boolean values.

The other issue with using exceptions in this way is that you are going to litter your code with unnecessary try-catch blocks that could potentially catch actual valid exceptions that you would want to know about. Returning a boolean does not prevent the calling code from throwing an exception if that is, in fact, the correct way to handle it. But throwing an exception means that any code that wants to reuse the logic here will need to wrap this in a try-catch block in order to interact with the result.

You should be writing your code for maximum reuse in order to get as much value from it as you can. In order to do this, CQS and saving exceptions for cases which are actually exceptional will do wonders in the long run.

The JavaScript Guard and Default Operators

There are two operators used in conditionals that most developers are very familiar with. Those are logical AND (&&) and logical OR (||).

These are short circuit operators, which means that they will not execute the second condition if it is not required.

if (isCondition1 && isCondition2)

If isCondition1 is false, there is no need to compute or check the value of isCondition2.

if (isCondition1 || isCondition2)

If isCondition1 is true, there is no need to compute or check the value of isCondition2.

In C#, each side of the operator is computed as a boolean value. But, in JavaScript, that is not the case. In JavaScript the type is coerced to a bool but not actually returned as a bool, which works as expected in conditionals because you’re not actually returning a value, just checking the boolean coercion..

if (objectA)

is valid in JavaScript, because if the object is not null or undefined, it will evaluate to true, but if the object is null or undefined it will be evaluated as false.

This means that you can perform evaluations such as:

if (objectA && objectA.isCondition1)

The operator short circuits if objectA is null or undefined, so the second condition is never even checked, preventing a null reference exception when checking isCondition1. Just in case you were wondering, this is a great habit to have. In C#, instead of just listing the object as the first check you would specifically have a null check.

You don’t have to save these operators for conditionals though. In C# you can use these operators outside of conditionals in order to assign a bool value to a variable. In JavaScript outside of a conditional, the type coercion is performed to see how to handle the result, but the value returned is not necessarily a boolean value. This means that these operators work differently than you might expect.

var someObject = objectA || {};

What does that statement do? It is the default operator. It means that if objectA would be coerced into false then the right side of the operator will act as a default value, in this case someObject will be an empty object. If objectA is defined, then someObject will be a reference to objectA.

var someObject = objectA && objectB;

This is the guard operator. If objectA would be coerced into false, then someObject will be equal to objectA, otherwise it will be equal to objectB. Outside of conditionals and avoiding null reference checks I get a lot less use out of this one.