Running with Code Like with scissors, only more dangerous

6Apr/141

The technical interview, part 2: Deck of Cards

Posted by Rob Paveza

Last time, I talked about how to dazzle an interviewer who asks you to write some code to test whether a given input is a palindrome. This time, it's my favorite interview question to ask.

Interviewer: Write some code that creates a deck of cards. If you ask for more details, I'll tell you something to the effect of, Imagine I want to write a game like poker or solitaire. I need to create a standard deck of cards.

This question is deliberately open-ended to test a few things:

  • Can you identify what kinds of objects are needed?
  • Can you design good APIs?
  • Can you choose effective data structures?
  • Can you design and implement good algorithms?

As a candidate, I would want to start out by establishing what cards are: they are a collection of a suit and a rank. These are each discrete values, and could be implemented as an enumeration in languages which support them, or else constants:

var Suit = {
    clubs: 1,
    diamonds: 2,
    hearts: 3,
    spades: 4
};
var Rank = {
    ace: 1,
    two: 2,
    three: 3,
    four: 4,
    five: 5,
    six: 6,
    seven: 7,
    eight: 8,
    nine: 9,
    ten: 10,
    jack: 11,
    queen: 12,
    king: 13
};
public enum Suit 
{
    Clubs,
    Diamonds,
    Hearts,
    Spades,
}

public enum Rank
{
    Ace,
    Two,
    Three,
    Four,
    Five,
    Six,
    Seven,
    Eight,
    Nine,
    Ten,
    Jack,
    Queen,
    King,
}
enum Suit {
    clubs,
    diamonds,
    hearts,
    spades,
}

enum Rank {
    ace,
    two,
    three,
    four,
    five,
    six,
    seven,
    eight,
    nine,
    ten,
    jack,
    queen,
    king,
}

Try this in the TypeScript playground.

An enumeration is a kind of type that provides a set of named constants. Not all languages support enumerations, but they're fairly easy to simulate in most, like I did in JavaScript above.

Now that we have the suit and rank set up, we can create a class or structure to represent a Card. Why have I chosen to do this? The problem asks for a deck of cards; clearly a Deck is a discrete object which is composed of some number of other discrete objects (Cards).

function Card(suit, rank) {
    if (suit < Suit.clubs || suit > Suit.spades)
        throw new RangeError('Invalid suit.');
    if (rank < Rank.ace || rank > Rank.king)
        throw new RangeError('Invalid rank.');

    this.suit = suit;
    this.rank = rank;
}
public class Card
{
    // The readonly modifier should only be used for immutable value-types.
    // In this case since they're enumerations which are value-types, it's good.
    private readonly Suit _suit;
    private readonly Rank _rank;

    public Card(Suit suit, Rank rank) 
    {
        // Checking range is also valid (like the other language examples).
        // This is a pretty good API to know about though.
        if (!Enum.IsDefined(typeof(Suit), suit))
            throw new ArgumentOutOfRangeException("suit");
        if (!Enum.IsDefined(typeof(Rank), rank))
            throw new ArgumentOutOfRangeException("rank");

        _suit = suit;
        _rank = rank;
    }

    public Suit Suit 
    {
        get { return _suit; }
    }

    public Rank Rank 
    {
        get { return _rank; }
    }
}
class Card
{
    // TypeScript supports a shorthand for declaring properties.
    // The code that this emits will be (almost) identical to the JavaScript sample,
    // except that the range check will be done after the property assignment.
    constructor(public suit: Suit, public rank: Rank) {
        if (suit < Suit.clubs || suit > Suit.spades)
            throw new RangeError('Invalid suit.');
        if (rank < Rank.ace || rank > Rank.king)
            throw new RangeError('Invalid rank.');
    }
}

Try this in the TypeScript playground.

In object-oriented programming, a class is the definition of a custom data type. Classes create an abstract "view" of data in the same way we can think of categories of objects (for example, "books" or "animals") which have some common sets of characteristics. Classes aren't the data themselves, however; we refer to individual datums as "objects" or "object instances." Each object of a given class should have the same kinds of data and similar behaviors, but each individual object might have specific nuances or individual data. For example, any given book might have a different number of pages, but all books have *some* number of pages.

Cards are simple data types. They have two properties - rank and suit - and once the card is created, you shouldn't be able to change those values. (The samples for JavaScript and TypeScript don't enforce that restriction, and if you point that out, you'll probably get bonus points). Given these constraints, it may also be worthwhile for the C# developer to point out that a Card could be a struct because structs should represent immutable values, but it's also not invalid to use a class here.

Classes have a desirable characteristic: they represent composition (or aggregation) of multiple values. That leads us to believe that a Deck, which is composed of multiple Cards, should also be a class:

function Deck() {
    this._cards = [];
}

Deck.prototype.addCard = function(card) {
    // Don't need to do checking specifically but if you want to be really fancy:
    if (!(card instanceof Card))
        throw new RangeError("Can only add a Card to the deck.");

    this._cards.push(card);
}

Deck.prototype.dealCard = function() {
    if (this._cards.length === 0)
        throw new RangeError("No cards to deal.");

    return this._cards.pop();
}

Deck.prototype.shuffle = function(numTimes) {
    // numTimes is an optional parameter.  If it's not a number, set a reasonable default.
    if (typeof numTimes !== 'number')
        numTimes = 5;

    var cards = this._cards;
    var cardCount = cards.length;

    // Do the shuffle operation numTimes times.
    for (var time = 0; time < numTimes; time++) {
        // Visit every card position once per "time"
        for (var index = 0; index < cardCount; index++) {
            // Create a random number in the range of [0, length)
            var numToSwap = Math.floor(Math.random() * cardCount);
            // Swap the cards at index and numToSwap
            var temp = cards[numToSwap];
            cards[numToSwap] = cards[index];
            cards[index] = temp;
        }
    }
}

// Not a prototype function.
Deck.createPokerDeck = function() {
    var result = new Deck();

    // Note these operators should be <= because we want to ensure we include all suits and ranks.
    for (var suit = Suit.clubs; suit <= Suit.spades; suit++) {
        for (var rank = Rank.ace; rank <= Rank.king; rank++) {
            var card = new Card(suit, rank);
            result.addCard(card);
        }
    }

    return result;
}
public class Deck
{
    private List<Card> _cards;
    private int _curPos;

    // The keen observer will notice that _curPos always equals _cards.Length - 1.
    // Can optimize this away but it's good for illustrative purposes.
    public Deck()
    {
        _cards = new List<Card>();
        _curPos = -1;
    }

    public void AddCard(Card card)
    {
        if (card == null)
            throw new ArgumentNullException("card");

        this._cards.Add(card);
        this._curPos++;
    }

    public Card DealCard()
    {
        if (this._curPos < 0 || this._cards.Count == 0)
            throw new InvalidOperationException("There are no cards to deal.");

        var card = this._cards[this._curPos];
        this._cards.RemoveAt(this._curPos);
        this._curPos--; // Optionally, decrement operation can be combined into the previous line as long as it's postfix.

        return card;
    }

    public void Shuffle(int numTimes = 5)
    {
        List<Card> cards = this._cards;
        int cardCount = cards.Count;
        Random rng = new Random();

        // Do the shuffle operation numTimes times.
        for (int time = 0; time < numTimes; time++)
        {
            // Visit every card position once per "time"
            for (var index = 0; index < cardCount; index++)
            {
                // Create a random number in the range of [0, length)
                int indexToSwap = rng.Next(cardCount);
                // Swap the cards at index and indexToSwap
                Card temp = cards[indexToSwap];
                cards[indexToSwap] = cards[index];
                cards[index] = temp;
            }
        }
    }

    public static Deck CreatePokerDeck()
    {
        Deck result = new Deck();

        // Note these operators should be <= because we want to ensure we include all suits and ranks.
        for (int suit = (int)Suit.Clubs; suit <= (int)Suit.Spades; suit++) {
            for (int rank = (int)Rank.Ace; rank <= (int)Rank.King; rank++) {
                var card = new Card((Suit)suit, (Rank)rank);
                result.AddCard(card);
            }
        }

        return result;
    }
}
class Deck
{
    private _cards: Array<Card>;
    constructor() {
        this._cards = [];
    }

    addCard(card: Card): void {
        // You can bypass runtime type checking here if you're using all TypeScript,
        // because the TypeScript compiler will emit a warning.  Otherwise, see 
        // the JavaScript sample for runtime type checking.

        this._cards.push(card);
    }

    dealCard(): Card {
        if (this._cards.length === 0)
            throw new RangeError('No cards to deal.');

        return this._cards.pop();
    }

    shuffle(numTimes: number = 5): void {
        var cards = this._cards;
        var cardCount = cards.length;

        // Do the shuffle operation numTimes times.
        for (var time = 0; time < numTimes; time++) {
            // Visit every card position once per "time"
            for (var index = 0; index < cardCount; index++) {
                // Create a random number in the range of [0, length)
                var numToSwap = Math.floor(Math.random() * cardCount);
                // Swap the cards at index and numToSwap
                var temp = cards[numToSwap];
                cards[numToSwap] = cards[index];
                cards[index] = temp;
            }
        }
    }

    static createPokerDeck(): Deck {
        var result = new Deck();

        // Note these operators should be <= because we want to ensure we include all suits and ranks.
        for (var suit = Suit.clubs; suit <= Suit.spades; suit++) {
            for (var rank = Rank.ace; rank <= Rank.king; rank++) {
                var card = new Card(suit, rank);
                result.addCard(card);
            }
        }

        return result;
    }
}

Try this in the TypeScript playground.

Let's unpack this piece-by-piece:

Use of an Array to store the Cards in JavaScript/TypeScript, List in C#

An Array is the default collection type in JavaScript and there isn't really a compelling reason to switch. Since an Array automatically grows and shrinks as you add and remove items, and these are such common operations that the engines optimize for them, there isn't a good reason to switch to another data type. In C#, the choice of a List is an interesting one. Alternatives include a linked list, Queue, or Stack, particularly because the JavaScript variants use Stack semantics (push/pop). However, the most computationally-intensive function of a Deck, Shuffle, requires random-access, and none of those alternatives is the right data type for random access.

AddCard

This method is pretty straightforward; it adds a new card to the end of the existing collection. You might call out that you're not checking for equality; this allows a Deck to contain multiple copies of the same value Card (for example, two Kings of Diamonds), for example. One thing that you might want to check for is reference equality (so that you're not adding two of the same exact Card variable, which would be impossible with real cards). However, that shouldn't be required unless you're really, really trying to dazzle your interviewer.

DealCard

This method is also pretty straightforward. It should take one card from one end of the array. In JavaScript, it's equally valid to use pop as shift, but I would balk at an implementation using splice as too complex. Range checking is a requirement for this method.

Basic operations and constructor

As an interviewer, I would expect a candidate to at minimum identified AddCard and DealCard as fundamental operations. I would strongly hope that the candidate would also have identified a need to shuffle the cards, and shuffling should also identify the need to have a basic set of cards to populate as well (or, the candidate may want a basic set of cards, and realize that they won't be shuffled). However, I would advise against populating the deck within the constructor. The Deck is the container of Cards; you use a Deck for Euchre, but it doesn't have the same set of Cards within it as a Deck for Texas hold 'em.

CreatePokerDeck

The most obvious way to implement this is to go through each of the ranks and suits, creating a Card for each, and adding it to an originally-empty Deck. There are a number of ways to do this; you might see the following implementation:

Deck.createPokerDeck = function () {
    var result = new Deck();

    var suits = [Suit.clubs, Suit.diamonds, Suit.hearts, Suit.spades];
    var ranks = [Rank.ace, Rank.two, Rank.three, Rank.four, Rank.five, Rank.six, Rank.seven, Rank.eight, Rank.nine, Rank.ten, Rank.jack, Rank.queen, Rank.king];
    suits.forEach(function (suit) {
        ranks.forEach(function (rank) {
            var card = new Card(suit, rank);
            result.addCard(card);
        });
    });

    return result;
}
    public static Deck CreatePokerDeck()
    {
        Deck result = new Deck();

        foreach (int suit in Enumerable.Range((int)Suit.Clubs, (int)Suit.Spades))
        {
            foreach (int rank in Enumerable.Range((int)Rank.Ace, (int)Rank.King))
            {
                var card = new Card((Suit)suit, (Rank)rank);
                result.AddCard(card);
            }
        }

        return result;
    }
    static createPokerDeck(): Deck {
        var result = new Deck();

        var suits: Array<Suit> = [Suit.clubs, Suit.diamonds, Suit.hearts, Suit.spades];
        var ranks: Array<Rank> = [Rank.ace, Rank.two, Rank.three, Rank.four, Rank.five, Rank.six, Rank.seven, Rank.eight, Rank.nine, Rank.ten, Rank.jack, Rank.queen, Rank.king];
        
        suits.forEach(suit => {
            ranks.forEach(rank => {
                var card = new Card(suit, rank);
                result.addCard(card);
            });
        });

        return result;
    }

Try this in the TypeScript playground.

I would raise a flag about complexity of this kind of implementation with the candidate. You might ask about performance and if there's a better way, particularly for the JavaScript versions, but also somewhat for the C# version. Each of these introduces new method calls which are really unnecessary, because you can get the same values just by doing addition.

Shuffle

Shuffle is my favorite problem of all. There are any number of ways to do it. It tests if candidates know about and understand how to use the random number generator in their language, and also lets you glimpse into reasoning. Almost always, candidates try to swap two random cards somewhere in the deck. The most obvious question to ask, though, is how do you know how many total times to actually perform swaps? You might shuffle 104 times for a 52-card deck, but even with that, there's a reasonable chance you might have never touched a particular card. Not that such an action is unreasonable; a card in a real deck might not change position either. But iterating through every card in a deck and then generating a random number is a sure way to ensure that every card is visited once.

One interesting pivot on this problem is using the JavaScript forEach function on your array, and then swapping. In general, I prefer not to see this; forEach implies an enumeration, and modifying an object being enumerated over is kind of a no-no. JavaScript engines apply a consistent behavior, but C# will barf on it (it's invalid to modify a collection while an enumerator is active on it in C#).

For JavaScript also, the developer needs to have clearly called Math.floor(Math.random() * count). Math.random() generates a double-precision floating point number in the range of [0, 1). Multiplying this value by the length of our array gives us a floating point number in the range of [0, length), but it'll still possibly have a fractional component; calling floor gives an integer in the range of [0, length-1].

Summary

This problem exercises basic problem-solving skills, some algorithms, some bits and bytes, and really focuses on object-oriented design skills. It also happens to be the exact problem I was given during my very first technical screening for a marketing agency in Phoenix.

Next time: Reasoning about linked lists.


3Apr/140

The technical interview, part 1: Palindrome

Posted by Rob Paveza

Last time, I talked about what I expect of an interview candidate as a technical job interviewer. This time, we're going to go through an initial question.

Interviewer: Write a function/method/subroutine to determine whether a particular string is a palindrome, i.e., is the same text when reversed as it is forward.

This is a common question for an interview because it allows you to demonstrate basic problem-solving skills and you can also show your interviewer that you know how to dive into the requirements. It requires that you can implement a simple algorithm, have an understanding of how strings work in your particular language, and can work through optimizations of your algorithm. Because the problem itself is so simple, you get a chance to dazzle your interviewer with good programming practices.

Understanding the problem

A palindrome is a string like kayak which you can reverse and end up with the same string. This gives you the naïve implementation:

    IsPalindrome(test)
        let reversed = ReverseString(test)
        return reversed equals test
    end

Most languages will have a baked-in function to reverse a string. So you could do that, but could you do it better?

Interviewer: Tell me about the performance characteristics of that baseline implementation.

You've allocated a new string, and depending on that particular implementation, it could have been costly. But you've got at least an O(n)* memory usage. String equality testing is going to be character-by-character, so that's going to be O(n) execution. So, if you're testing a string that has 2 million characters, the naïve algorithm allocates an additional 2mb string and does (in theory) up to 2 million comparisons.

How can we optimize? Well, let's start with understanding the basic problem again. Let's consider kayak:

k a y a k
0 1 2 3 4

Given this string layout, what would we need to do to go faster?

  1. Compare 0 to 4. Match, continue.
  2. Compare 1 to 3. Match, continue.
  3. Only one uncompared character is remaining, therefore return true.

This can be generalized to an algorithm:

  1. Initialize start and end indices to 0 and length - 1 (or 1 and length for 1-based index programming languages).
  2. Compare characters at the start and end indices. If they don't match, return false (fast exit).
  3. Increment start and decrement end.
  4. If the end index is less than or equal to start, return true. Otherwise, loop back to the step 2.

We now know enough to author a simple method to do this:

function isPalindrome(stringToTest) {
    var start = 0, end = stringToTest.length - 1;
    for (/* already initialized */ ; start < end; start++, end--) {
        if (stringToTest[start] !== stringToTest[end])
            return false;
    }
    // got to the end with no mismatches, implies success
    return true;
}
class Utilities 
{
    public static bool IsPalindrome(string toTest) 
    {
        int start = 0, end = toTest.Length;
        for (/* already initialized */ ; start < end; start++, end--)
        {
            if (toTest[start] != toTest[end])
                return false;
        }
        // got to the end with no mismatches, implies success
        return true;
    }
}
bool isPalindrome(const wchar_t* toTest const) {
    auto len = wcslen(toTest);
    wchar_t* start = const_cast<wchar_t*>(toTest);
    wchar_t* end = start + len - 1;
    for (/* already initialized */ ; start < end; start++, end--) {
        if (*start != *end)
            return false;
    }
    return true;
}

This is a fairly optimal solution.

Interviewer: What about error cases? What kind of error cases exist, and how can you work around them?

In JavaScript, because we don't do compile-time type validation, it's possible for types to mismatch. A caller can pass in a number, an object, undefined, no value, null, a Boolean, etc. Each of these has different odd behaviors; for an Array, for example, the behavior will effectively match how things work with strings. But, for most other values, because there isn't any string coercion, and math is performed directly against the value's length property (which will generally result in NaN), and therefore that will return true. isPalindrome(true) definitely should return false, but it doesn't!

There are a couple of ways to guard against this. You can test the input parameter for its type and fail with a TypeError. You could combine this by attaching the method to the String prototype, which would enable the function to be called against any String object as an instance method.

In C#, if you pass in a null reference, the method will fail with a NullReferenceException on the first dereference of toTest.Length. This is acceptable but likely not ideal; instead, it would be preferable to fail with an ArgumentNullException. You can also show nice API design intuition by making it into an extension method.

If you're authoring in C++, a great question to ask would be the kind of error environment you need to deal with. In COM, for example, you don't want to use structured exception handling, or at least allow SEH exceptions to cross the ABI boundary. Depending on your environment, you may be able to use SEH, but be prepared to author a COM version of the API. Of course, COM also advises a different type system. COM strings tend to be BSTRs, where a null value is equivalent to a zero-length string. (WinRT uses HSTRINGs, which behave similarly to BSTRs).

String.prototype.isPalindrome = function() { // argument removed, it is "this"
    if (typeof this !== 'string' && !(this instanceof String)) // In case of call, apply, or bind
        throw new TypeError('Can only call against a string.');

    var start = 0, end = this.length - 1;
    for (/* already initialized */ ; start < end; start++, end--) {
        if (this[start] !== this[end])
            return false;
    }
    // got to the end with no mismatches, implies success
    return true;
}
static class StringExtensions
{
    public static bool IsPalindrome(this string toTest) 
    {
        if (toTest == null)
            throw new ArgumentNullException("toTest");

        int start = 0, end = toTest.Length;
        for (/* already initialized */ ; start < end; start++, end--)
        {
            if (toTest[start] != toTest[end])
                return false;
        }
        // got to the end with no mismatches, implies success
        return true;
    }
}
// This sample is using WRL and Windows Runtime (because it's the C++ COM library that I know)
// IDL file:
namespace MyApi 
{
    [uuid(...)]
    [version(...)]
    interface IPalindromeTest 
    {
        HRESULT IsPalindrome([in] HSTRING toTest, [out, retval] boolean* result);
    }
}

// header:
using namespace ABI::MyApi;
class PalindromeTest : public RuntimeClass<IPalindromeTest>
{
    InspectableClass(InterfaceName_MyApi_IPalindromeTest, BaseTrust)

public:
    PalindromeTest();
    virtual ~PalindromeTest();
    HRESULT RuntimeClassInitialize();

    IFACEMETHOD(IsPalindrome)(_In_ HSTRING toTest, _Out_ boolean* result);
}

// implementation:
// Other stuff - RuntimeClassInitialize, etc. - goes here
IFACEMETHODIMP PalindromeTest::IsPalindrome(_In_ HSTRING toTest, _Out_ boolean* result)
{
    IfNullReturnError(result, E_POINTER);
    HRESULT hr;
    size_t len;
    const wchar_t* strVal;

    strVal = WindowsGetStringRawBuffer(toTest, &len);
    wchar_t* start = const_cast<wchar_t*>(strVal);
    wchar_t* end = start + len - 1;
    for (/* already initialized */ ; start < end; start++, end--) {
        if (*start != *end) 
        {
            *result = false;
            return S_OK;
        }
    }
    *result = true;
    return S_OK;
}

Interviewer: This is a pretty good solution. What are some of the pitfalls or problems with it? What are some possible variations that you might do?

This is an open-ended question intended to see what you might consider for your implementation and also to gauge what you know and maybe don't know. Possible answers include:

This implementation fails to work with Unicode characters above U+FFFF. JavaScript, modulo ES6, doesn't support characters outside of the UCS-2 range. They can go back and forth to a host which does, but intrinsically, JavaScript didn't understand the idea of surrogate pairs. C++'s wchar_t and C#'s string don't directly support surrogate pairs because the underlying character type is 16-bit. In order to fix the code, you'd need to get the "real" length of the strings (factoring in the code points) and then going code-point-by-code-point instead of index-by-index.

I could implement this function recursively instead of iteratively. Generally, this implementation is better as an iterative function than a recursive function. But recognizing that it's possible, because the algorithm is really about solving progressively simple sub-problems, is a good thing to do. In languages which support tail-call recursion, this can be as well-performing as the iterative implementation.

(function() {
    function isPalindrome(toCheck, startIndex, endIndex) {
        if (endIndex >= startIndex)
            return true;

        if (toCheck[startIndex] !== toCheck[endIndex])
            return false;

        return isPalindrome(toCheck, startIndex + 1, endIndex - 1);
    }

    String.prototype.isPalindrome = function() { 
        if (typeof this !== 'string' && !(this instanceof String)) 
            throw new TypeError('Can only call against a string.');

        return isPalindrome(this, 0, this.length - 1);
    }
})();

Interviewer: Great. How would you validate this implementation?

This gives you a few options for approaches. This problem is simple enough that you could intuit the sets of cases:

  • A null value
  • A zero-length string
  • An odd-length string
  • An even-length string

You might also walk through your code to identify all of the different code paths. Doing so will help you get to the same set of cases.

Summary

If you've talked through all of this, then chances are I already think you're a pretty strong candidate. This isn't a complete list of things I might ask you to talk about, but I'm hoping it's taken 15-20 minutes, and we can talk about more next.

Up next: a deck of cards.


* O(n) notation is a mechanism by which we describe the worst-case performance characteristics of a system. For memory, it's referring to the number of bytes allocated; for CPU utilization, it's the number of times a particular loop operation must be executed. It's generally described as the largest factor in the equation; O(n) is linear time, O(n2) is polynomial time, O(log n) is logarithmic time, etc. This is a simplification, but I'd suggest having a strong grasp of this concept.


6Feb/120

In a disconnected world, robust code is crucial

Posted by Rob Paveza

Probably 99.99% of HTML applications and websites are served over HTTP exclusively. (I'm referring here to HTTP as a transport protocol, not HTTP vs.HTTPS, for example, and I realize that HTTP is an application-layer protocol according to OSI; but developers generally treat it as an abstraction for "the network"). As anybody who has done web programming knows, HTTP is a stateless protocol; that is, it's based on a request-response model, and in general, one request has no knowledge of previous requests. This has posed some challenges for web developers over the years, and some brilliant abstractions of state on top of the statelessness have been devised.

The hard part now, though, isn't to deal with statelessness. It's dealing with the request-and-response model.

All network communication is inherently request-and-response. There are some applications that utilize full-duplex communications to get around that (think of chat software), but for the most part, that isn't really available behind the firewall. Web sockets are still yet to be standardized (and there are some questions about long-term compatibility with WebSocket-ignorant proxies). And typically, corporate firewalls say no to outbound connections except on ports 80 or 443. Some applications (think Meebo) have been able to get around this limitation by cleverly using long-timeout delays on AJAX requests. The client makes a request to the server, and the server either responds immediately (if an event is in queue) or holds the request for 30-90 seconds to see if an event comes in. I even did this once myself with good success, although I never took that app into production. (There was also some question about the total # of clients an ASP.NET server could sustain whilst holding threads in that way).

In many respects, Windows developers haven't had to deal with this. We could issue synchronous requests, and the UI would stand still for a second, and either it would work or it would fail. But usability concerns over this process, as well as issues with high network latency (imagine pressing the "Submit" button and having to wait 20 seconds while your app freezes - by then, I've force-closed the app) have seen platform providers decree that asynchrony is the only way to go.

HTML isn't the only application provider dealing with this limitation. Adobe Flash has long had an asynchronous-communication-only model, Microsoft Silverlight has also carried on this principle; of course, these two applications have lived predominantly in browsers, where a hanging UI probably means interfering with other apps as well as the one making the request. Interestingly, WinRT - the Windows 8 developer framework - is also going to mandate an asynchronous model, following in the Silverlight-based foodsteps blazed by Windows Phone 7.

So as we trek out into the world of asynchrony, well, we have a whole mess of questions to deal with now:

  • If there's an error, does it show up in the calling method or in the callback method? Does it even show up?
  • Does a network (transport-level) error surface differently than an application error? What if the server returned an HTTP 403 Forbidden response?
  • What are all of the different kinds of errors that can crop up? Do I need to handle SocketException or is that going to be abstracted to something more meaningful to my application?
  • What do I do if a network error comes up? Do I assume that I'm offline, panic, and quit? What if my application only makes sense "online"?
  • Do I surface an error to the customer? Silently fail? I might generally fail silently if I'm a background process, but then again, what if it's an important one? What if the customer thought he was saving his draft while all along it was offline, and then the customer closes the browser?
  • During the async operation, should I show the user a timeout spinner or something to that effect?
  • How should I design my async operations? For example, consider a Save operation. Should I capture all of my state at once and send it off, and let the user immediately keep working? Should I make the user wait until saving completes? Should I even use Save, or automatically save whenever something changes?
  • If I use auto-save, how do I handle undo? What if I want to undo between sessions? Is there a way to go back if the hosting application crashes? (Worst case scenario: the user accidentally hit Select All, Delete and then the browser crashed after the auto-save).

This merely scratches the surface of the kinds of questions we'll need to begin asking ourselves. It doesn't even deal with the difficulties of programming asynchronously, which C# 5 is going to deal with extraordinarily, but many developers will be unable to take advantage of these updates. For example, suppose I have a widget on my page that monitors the status of a long-running server-based process. I need to have JavaScript on my page that monitors that process and updates my widget accordingly. Should I:

  • Write a singleton object? This might be easier and afford strong member protection, but I can only have one widget, unless I somehow differentiate between them and multiplex, which can become hairy quickly.
  • Should the monitoring function accept a callback, or should it be event-based, so that multiple subscribers can listen? (Maybe an event-based model offers some interesting ways to deal with the complexities of a singleton?)
  • Should the widget manipulate the view directly, or should I write separate code that handles the view based on the state of the object (or objects)?

The list goes on.

We're moving faster and faster into an asychronous world. It is already happening, and we as developers need to be prepared to handle these difficulties. We also need to understand how to communicate these kinds of questions to our business analysts, our supervisors, and our customers. We need to be able to equip ourselves to ask the right questions of our customers, so that when it's time to make a decision, we have the information we need.

9Jan/121

Revealing Prototype Pattern: Pros and Cons

Posted by Rob Paveza

A short while ago, I wrote a post generally saying good things about the Revealing Prototype Pattern but mostly focused tearing down the other part that was presented with it, namely the way that variables were declared in a chain separated by the comma operator. This post will discuss some of the pros and cons of using this pattern, and give some examples about when you should or shouldn't use it.

Advantages

As Dan notes in his post, this features a significant improvement over straight prototype assignment (assignment of an object literal to a prototype), in that private visibility is supported. And because you're still assigning an object to the prototype, you are able to take advantage of prototypal inheritance. (Next time: How prototypal inheritance really works and how it can make your head explode).

Except for chaining his variable declarations, I have to admit Dan had me pretty well sold on the Revealing Prototype Pattern. It's very elegant; it provides good protection to its inner variables, and it uses a syntax we've been seeing more and more of ever since jQuery became a popular platform.

Unfortunately, it has some nasty drawbacks.

Disadvantages

To be fair, Dan lists some of the disadvantages about this pattern; however, he doesn't quite list them as such, and I think he was possibly unaware of some of their implications:

There's something interesting that happens with variables though, especially if you plan on creating more than one Calculator object in a page. Looking at the public functions you'll see that the 'this' keyword is used to access the currNumberCtl and eqCtl variables defined in the constructor. This works great since the caller of the public functions will be the Calculator object instance which of course has the two variables defined. However, when one of the public functions calls a private function such as setVal(), the context of 'this' changes and you'll no longer have access to the two variables.

The first time I read through that I glossed over the problems; I didn't quite understand the issue until I wrote some code. So let's do that - we'll implement the Java StringTokenizer class:

function StringTokenizer(srcString, delim)
{
    if (typeof srcString === 'undefined')
        throw new ReferenceError("Parameter 0 'srcString' is required.");
    if (typeof srcString !== 'string')
        srcString = srcString.toString();
    if (typeof delim !== 'string')
        delim = ' ';
    
    if (!(this instanceof StringTokenizer))    // enforce constructor usage
        return new StringTokenizer(srcString, delim);
        
    this.sourceString = srcString;
    this.delimiter = delim;
}
StringTokenizer.prototype = (function()
{
    var that = this;
    
    var _tokens = that.sourceString.split(that.delimiter);
    var _index = 0;
    
    var _countTokens = function() { return _tokens.length; };
    var _hasMoreTokens = function() { return _index < _tokens.length; };
    var _nextToken = function()
    {
        if (!_hasMoreTokens())
            return false;
        
        var next = _tokens[_index];
        _index += 1;
        return next;
    };
    var _reset = function() { _index = 0; };
    
    var resultPrototype = 
    {
        countTokens: _countTokens,
        hasMoreTokens: _hasMoreTokens,
        nextToken: _nextToken,
        reset: _reset
    };
    return resultPrototype;
})();

If you've ever written a jQuery plugin, you'll probably recognize what I did with the prototype assignment function; when writing jQuery plugins, it's common to close over the current instance of the jQuery object by assigning var that = $(this); so that you can write event-handler functions without losing access to the overall context. Unfortunately, what I did in this case is wrong; you may already see why.

var that = this;

In this context, this is a reference to the global object, not to the instance of the object - even though the prototype is being set. This is a generalization of what Dan said. Rewriting it to overcome it results in information leaking:

function StringTokenizer(srcString, delim)
{
    if (typeof srcString === 'undefined')
        throw new ReferenceError("Parameter 0 'srcString' is required.");
    if (typeof srcString !== 'string')
        srcString = srcString.toString();
    if (typeof delim !== 'string')
        delim = ' ';
    
    if (!(this instanceof StringTokenizer))    // enforce constructor usage
        return new StringTokenizer(srcString, delim);
        
    this.sourceString = srcString;
    this.delimiter = delim;
    this.tokens = srcString.split(delim);
    this.index = 0;
}
StringTokenizer.prototype = (function()
{
    var _countTokens = function() { return this.tokens.length; };
    var _hasMoreTokens = function() { return this.index < this.tokens.length; };
    var _nextToken = function()
    {
        if (!this.hasMoreTokens())
            return false;
        
        var next = this.tokens[this.index];
        this.index += 1;
        return next;
    };
    var _reset = function() { this.index = 0; };
    
    var resultPrototype = 
    {
        countTokens: _countTokens,
        hasMoreTokens: _hasMoreTokens,
        nextToken: _nextToken,
        reset: _reset
    };
    return resultPrototype;
})();

The code works correctly; but you can see that we have to make public all of the state variables we'll use in the constructor. (The alternatives are to either initialize the state variables in each function, where they would still be public; or to create an init function, which would still cause the variables to be public AND would require the user to know to call the init function before calling anything else).

Dan also indicated that you needed a workaround for private functions:

There are a few tricks that can be used to deal with this, but to work around the context change I simply pass “this” from the public functions into the private functions.

Personally, I prefer to try to avoid things one might call clever or tricky, because that's code for "so complex you can't understand it". But even in the case where you have a public function, you'll still get an error if you don't reference it via a public function call. This error is nonintuitive and could otherwise make you go on a bug hunt for a long time. Consider this change to the above code:

    var _hasMoreTokens = function() { return this.index < this.tokens.length; };
    var _nextToken = function()
    {
        if (!_hasMoreTokens())   // changed from:   if (!this.hasMoreTokens())
            return false;
        
        var next = this.tokens[this.index];
        this.index += 1;
        return next;
    };

Simply removing the 'this' reference in the caller is enough to cause 'this' to go out-of-scope in the _hasMoreTokens function. This is completely unintuitive behavior for developers who grew up in the classical inheritance model.

Alternatives

I wouldn't want to give you all of these options without giving you an alternative. The alternative I present here is one in which the entire object is populated in the constructor:

"use strict";
function StringTokenizer(srcString, delim)
{
    if (typeof srcString === 'undefined')
        throw new ReferenceError("Parameter 0 'srcString' is required.");
    if (typeof srcString !== 'string')
        srcString = srcString.toString();
    if (typeof delim !== 'string')
        delim = ' ';
    
    if (!(this instanceof StringTokenizer))    // enforce constructor usage
        return new StringTokenizer(srcString, delim);
        
    if (typeof Object.defineProperty !== 'undefined')
    {
        Object.defineProperty(this, 'sourceString', { value: srcString });
        Object.defineProperty(this, 'delimiter', { value: delim });
    }
    else
    {
        this.sourceString = srcString;
        this.delimiter = delim;
    }
    
    var _tokens = this.sourceString.split(this.delimiter);
    var _index = 0;
    
    var _countTokens = function() { return _tokens.length; };
    var _hasMoreTokens = function() { return _index < _tokens.length; };
    var _nextToken = function()
    {
        if (!_hasMoreTokens())
            return false;
        
        var next = _tokens[_index];
        _index += 1;
        return next;
    };
    var _reset = function() { _index = 0; };
    
    if (typeof Object.defineProperty !== 'undefined')
    {
        Object.defineProperty(this, 'countTokens', { value: _countTokens });
        Object.defineProperty(this, 'hasMoreTokens', { value: _hasMoreTokens });
        Object.defineProperty(this, 'nextToken', { value: _nextToken });
        Object.defineProperty(this, 'reset', { value: _reset });
    }
    else
    {
        this.countTokens = _countTokens;
        this.hasMoreTokens = _hasMoreTokens;
        this.nextToken = _nextToken;
        this.reset = _reset;
    }
}

The advantage of a structure like this one is that you always have access to this. (Note that this example is unnecessarily large because I've taken the additional step of protecting the properties with Object.defineProperty where it is supported). You always have access to private variables and you always have access to the state. The unfortunate side effect of this strategy is that it doesn't take advantage of prototypal inheritance (it's not that you can't do it with this strategy - more of that coming in the future) and that the entire private and public states (including functions) are closed-over, so you use more memory. Although, one may ask: is that really such a big deal in THIS sample?

Usage Considerations

The Revealing Prototype Pattern can be a good pattern to follow if you're less concerned with maintaining data integrity and state. You have to be careful with access non-public data and functions with it, but it's pretty elegant; and if you're working on a lot of objects, you have the opportunity to save on some memory usage by delegating the function definitions into the prototype rather than the specific object definition. It falls short, though, when trying to emulate classical data structures and enforce protection mechanisms. As such, it can require complicated or clever tricks to work around its shortcomings, which can ultimately lead to overly-complex or difficult-to-maintain code.

Like most patterns, your mileage may vary.

6Jan/120

Defining Variables in JavaScript

Posted by Rob Paveza

I've lately been reviewing different patterns and practices recently, and after reading his article about the Revealing Prototype Pattern, I wanted to take some time to analyze Dan Wahlin's approach to defining variables. It's hard to believe I found some common ground with Douglas Crockford, but as they say about broken clocks.... [Addendum: apparently, since JSlint says to 'combine with previous var statement,' I don't agree with Crockford.] Anyway, to begin, this post is inspired by Dan Wahlin's presentation of the Revealing Prototype Pattern; I noticed what I thought was a curious way for him to define his private variables, and looking back through his blog posts he discussed it in the original blog post of the series, Techniques, Strategies, and Patterns for Structuring JavaScript Code. For the most part, I like what Dan has to say, but I'm going to have to disagree when it comes to defining variables.

The Proposition

As Dan points out, this is the standard way of defining variables in JavaScript:

var eqCtl;
var currNumberCtl;
var operator;
var operatorSet = false;
var equalsPressed = false;
var lastNumber = null;

He advocates trading that for this:

var eqCtl,
    currNumberCtl,
    operator,
    operatorSet = false,
    equalsPressed = false,
    lastNumber = null;

It saves on 20 keystrokes, and he claims improved readability. Now, I disagree with Crockford's argument that, because JavaScript hoists variables to the top of the function, that you should always declare the variable there. I believe that, whenever possible, you should try to maximize locality of a variable. This is a principle discussed in Steve McConnell's Code Complete; the reasoning behind maximization of locality is that the human brain can only comprehend so much at once. (This is, of course, another argument in favor of many, simple, and small subroutines). By delaying the declaration of a variable until it needs to be used, we are able to better-comprehend the meaning of the variable and how its use affects and relates to the rest of the program. As such, I believe that one of the premises for moving these declarations into a combined var statement - specifically, to reflect the hoisting - is a poor rationale.

Let's carry on.

Similarities to Other Elements

In The Prototype Pattern, Dan demonstrates the use of a JavaScript object literal in assignment to the Calculator prototype, so that any object created using the Calculator constructor would inherit all of those properties:

Calculator.prototype = {
    add: function (x, y) {
        return x + y;
    },
    subtract: function (x, y) {
        return x - y;
    },
    multiply: function (x, y) {
        return x * y;
    },
    // ...
};

The important thing to note here is that we are simply defining an object literal; we are not writing procedural code, and that comma is not an operator! It is an important part of the JavaScript grammar, to be sure, but the comma here does not have the same semantic meaning as the comma we saw before. This subtle difference may lead to coding errors, in which someone who uses comma syntax with both will mistakenly believe they are declaring an object literal and use colons to separate the identifier from the value; or that they are declaring variables and use assignment syntax to separate the property from its value.

Comma Operator Considered Harmful

It surprises me to learn that JSlint advocates combining var declarations. Crockford's The Elements of JavaScript Style, Part 1 indicates that he isn't terribly fond of it either:

The comma operator was borrowed, like much of JavaScript's syntax, from C. The comma operator takes two values and returns the second one. Its presence in the language definition tends to mask certain coding errors, so compilers tend to be blind to some mistakes. It is best to avoid the comma operator, and use the semicolon statement separator instead.

Whichever way Crockford prefers it, I think what we need to remember is just because you CAN do something does not mean you SHOULD.

Let's consider Dan's full body of JavaScript from the Revealing Prototype Pattern. I'm going to shrink it a little bit, to emphasize the changes I'll make; and I'm removing any of his comments.

var Calculator = function (cn, eq) {
    this.currNumberCtl = cn;
    this.eqCtl = eq;
};

Calculator.prototype = function () {
    var operator = null,
        operatorSet = false,
        equalsPressed = false,
        lastNumber = null,
        add = function (x, y) { return x + y; },
        subtract = function (x, y) { return x - y; },
        multiply = function (x, y) { return x * y; },
        // I'm going to do something evil here.
        divide = function (x, y) {
            if (y == 0) {
                alert("Can't divide by 0");
            }
            return x / y;
        },
        setVal = function (val, thisObj) { thisObj.currNumberCtl.innerHTML = val; },
        setEquation = function (val, thisObj) { thisObj.eqCtl.innerHTML = val; },
        clearNumbers = function () {
            lastNumber = null;
            equalsPressed = operatorSet = false;
            setVal('0',this);
            setEquation('',this);
        },
        setOperator = function (newOperator) {
            if (newOperator == '=') {
                equalsPressed = true;
                calculate(this);
                setEquation('',this);
                return;
            }
            if (!equalsPressed) calculate(this);
            equalsPressed = false;
            operator = newOperator;
            operatorSet = true;
            lastNumber = parseFloat(this.currNumberCtl.innerHTML);
            var eqText = (this.eqCtl.innerHTML == '') ?
                lastNumber + ' ' + operator + ' ' :
                this.eqCtl.innerHTML + ' ' + operator + ' ';
            setEquation(eqText,this);
        },
        numberClick = function (e) {
            var button = (e.target) ? e.target : e.srcElement;
            if (operatorSet == true || 
                this.currNumberCtl.innerHTML == '0') {
                setVal('', this);
                operatorSet = false;
            }
            setVal(this.currNumberCtl.innerHTML + button.innerHTML, this);
            setEquation(this.eqCtl.innerHTML + button.innerHTML, this);
        },
        calculate = function (thisObj) {
            if (!operator || lastNumber == null) return;
            var displayedNumber = parseFloat(thisObj.currNumberCtl.innerHTML),
                newVal = 0;
            switch (operator) {
                case '+':
                    newVal = add(lastNumber, displayedNumber);
                    break;
                case '-':
                    newVal = subtract(lastNumber, displayedNumber);
                    break;
                case '*':
                    newVal = multiply(lastNumber, displayedNumber);
                    break;
                case '/':
                    newVal = divide(lastNumber, displayedNumber);
                    break;
            }
            setVal(newVal, thisObj);
            lastNumber = newVal;
        };
    return {
        numberClick: numberClick,
        setOperator: setOperator,
        clearNumbers: clearNumbers
    };
} ();

Note my comment: "I'm going to do something evil here." Here goes:
console.log('Hello, world')

Do you see what happened here? Let me put it in context.

        subtract = function (x, y) { return x - y; },
        multiply = function (x, y) { return x * y; },
        console.log('Hello, world')
        divide = function (x, y) {
            if (y == 0) {
                alert("Can't divide by 0");
            }
            return x / y;
        },

JavaScript semicolon insertion blew away the var when I inserted any valid statement or expression. In fact, I could have simply put 'Hello, world!' or 5 there, on its own line, and because the next line is a valid statement that stands on its own, JavaScript semicolon insertion blew away the var. As such, divide, setVal, setEquation, clearNumbers, setOperator, numberClick, and calculate were all just elevated to global scope, possibly blowing away existing variables and leaking a whole bunch of information with them. This could happen in any instance in which someone mistakenly types a semicolon (let's be honest - JavaScript is a semicolon-terminated language; it will happen somewhat frequently), or if they forget to put a comma at the end of a line.

As such, joining variable declarations together by using the comma operator is inherently an unsafe operation. You might think of it as a run-on sentence; it's not a good thing to do in English, so why would it be good to do in JavaScript or any other programming language?

And if that's not reason enough, here's another: declaring a variable is a statement. You are stating to the compiler, "I am declaring this variable to be a variable." Use the var statement to make a statement, and use the comma operator to indicate that there are operations. (Specifically, the one and only place I can think of in which a comma operator would be appropriate is if you need a single for-loop with multiple iterator variables, e.g., for (var i = 0, j = myArray.length - 1; i = 0; i++, j--). Of course, I don't want to say you should never use it, or else I'd be like Crockford with his dogmatic "I have never seen a piece of code that was not improved by refactoring it to remove the continue statement," which is patently silly.

But, beware the comma. He is correct in that it is easy to mark programming errors with commas. If you're going to declare a variable, do everyone a favor and declare it, using var, make it feel special by giving it its own line and declaration and semicolon. It will help in maintenance down the line.

18Mar/080

Thoughts on Generics and Best Practices – Building Cohesive Software

Posted by Rob

In general, I don't know whether I internalized this as a best practice because it makes sense to me, or because I read it somewhere.  But I have this notion that there are only certain places in which it is appropriate to expose a generic member to the outside world, such as when the item is clearly generic in scope.  For example, if I wanted to implement a FIFO list to get the behavior of a queue but the performance characteristics of a list, I might call my class ListQueue<T>.  As a library class, it makes sense to have this as a generic type.  But what about when it would save programmer effort only?  I don't think that this is a valid reason to expose the class to the outside world.

Between my last two projects, I have about a dozen classes that look like this:

   1:      [ConfigurationCollection(typeof(SerialPortConfigurationElement), AddItemName = "SerialPort", CollectionType = ConfigurationElementCollectionType.BasicMap)]
   2:      public class SerialPortConfigurationElementCollection : ConfigurationElementCollection, IEnumerable<SerialPortConfigurationElement>
   3:      {
   4:          protected override ConfigurationElement CreateNewElement()
   5:          {
   6:              return new SerialPortConfigurationElement();
   7:          }
   8:   
   9:          protected override object GetElementKey(ConfigurationElement element)
  10:          {
  11:              return ((SerialPortConfigurationElement)element).PortName; 
  12:          }
  13:   
  14:          public override ConfigurationElementCollectionType CollectionType
  15:          {
  16:              get
  17:              {
  18:                  return ConfigurationElementCollectionType.BasicMap;
  19:              }
  20:          }
  21:   
  22:          public SerialPortConfigurationElement this[int index]
  23:          {
  24:              get { return BaseGet(index) as SerialPortConfigurationElement; }
  25:          }
  26:   
  27:          public SerialPortConfigurationElement this[string index]
  28:          {
  29:              get { return BaseGet(index) as SerialPortConfigurationElement; }
  30:          }
  31:   
  32:          #region IEnumerable<SerialPortConfigurationElement> Members
  33:   
  34:          public new IEnumerator<SerialPortConfigurationElement> GetEnumerator()
  35:          {
  36:              return new LightWrapperEnumerator<SerialPortConfigurationElement>(base.GetEnumerator());
  37:          }
  38:   
  39:          #endregion
  40:      }

This class enables the following type of configuration settings:

   1:      <SerialPorts>
   2:          <SerialPort PortName="COM6" BaudRate="9600" DataBits="8" Parity="None" StopBits="One" />
   3:          <SerialPort PortName="COM8" /> <!-- Only the PortName is required.  The others have defaults. -->
   4:      </SerialPorts>

This is all well and good.  However, as I look at my class, I see that it would be fairly easy to rip out a lot of that functionality and make it a generic class.  LightWrapperEnumerator<T> is already something like that - it's an internal-only class that basically wraps an IEnumerator into an IEnumerator<T>.  That's fairly easy to hide from the outside world, though; I only expose it to calling code using IEnumerator<T> and nobody is the wiser unless they call GetType() on it, which is unlikely.  I realized as I built the second duplicate of this class today involving a Motor configuration element, that I could rip out the guts and implement everything EXCEPT GetElementKey and have it out there:

   1:  abstract class ChildlessConfigurationElementCollection<T> : ConfigurationElementCollection 
   2:      where T : ConfigurationElement, new()
   3:  {
   4:      protected override ConfigurationElement CreateNewElement()
   5:      {
   6:          return new T();
   7:      }
   8:   
   9:      public override ConfigurationElementCollectionType CollectionType
  10:      {
  11:          get
  12:          {
  13:              return ConfigurationElementCollectionType.BasicMap;
  14:          }
  15:      }
  16:   
  17:      public T this[int index]
  18:      {
  19:          get { return BaseGet(index) as T; }
  20:      }
  21:   
  22:      public T this[string index]
  23:      {
  24:          get { return BaseGet(index) as T; }
  25:      }
  26:   
  27:      #region IEnumerable<SerialPortConfigurationElement> Members
  28:   
  29:      public new IEnumerator<T> GetEnumerator()
  30:      {
  31:          return new LightWrapperEnumerator<T>(base.GetEnumerator());
  32:      }
  33:   
  34:      #endregion
  35:  }

Implementing the serial port collection then, would be VERY easy:

   1:  [ConfigurationCollection(typeof(SerialPortConfigurationElement), AddItemName = "SerialPort", CollectionType = ConfigurationElementCollectionType.BasicMap)]
   2:  class SerialPortConfigurationElementCollection : ChildlessConfigurationElementCollection<SerialPortConfigurationElement>
   3:  {
   4:      protected override object GetElementKey(ConfigurationElement element)
   5:      {
   6:          return ((SerialPortConfigurationElement)element).PortName;
   7:      }
   8:  }

However, aside from the fact that they have the same "programmer functionality," there's not much motivating me to do anything else.

Hopefully, as you're looking at these two examples, you're saying "that doesn't look correct."  From an object model perspective, it doesn't make much sense, even in the abstract world (away from objects), to have even a concept of a "Childless Configuration Element Collection."  What does it mean, anyway?  Would it make sense for me to cast a configuration property object to a ChildlessConfigurationElementCollection of T?  I... don't think so.

I've been reading Code Complete 2nd Ed. lately.  I can't remember whether I've blogged about my respect for Steve McConnell before, but this guy is simply an amazing contributor to our craft.  In it, he talks at length about a software quality attribute that we'd never really discussed in school: cohesion.

The Wikipedia article gives it a great treatment, so I won't really delve into the depths of what cohesion means and how it can be measured (buy the book!).  As I look at this type of code, though, I can only see some kind of cross between procedural and logical cohesion - not exactly excellent in the scale.  The real bottom line is that these all operate on different data - how are we to treat them the same?

Incidentally, the way that C++ implements templates (not C++/CLI generics) would make this not be so terrible.  I could define this template class and it would actually generate the C++ code (Java actually would do this in an adequate way, as well).  Because the code is generated at compile-time, I don't need to expose the metadata about the template class, and so I'm saved in that regard.  Unfortunately, if I were to define this class in C# and need to access it in an external assembly, I'd need to mark the base class as public, and so everything else in the world can inherit from or create references to variables of type ChildlessConfigurationElementCollection<T>.  It doesn't make sense to have this type exposed; it only makes sense at the higher level. 

So I am left with the decision: do I duplicate code or expose bad, incomplete objects to the world?  I'll duplicate code, thank you very much.

3Mar/080

Exceptional Exception Handling: Taking Exception with the Community

Posted by Rob

I could have gone on with the title for a while longer.

Frequently when I'm working on an application, it occurs to me that the user doesn't care about the technical reasons behind an exceptional condition.  Typically, the user simply cares that something went wrong; or, sometimes, the user doesn't care, especially if you can make the program recover gracefully.

Since the only .NET-sanctioned way to handle errors is through structured exception handling, and since I love SEH, I don't want to be too hard on it.  But the truth is, there are certainly instances where "On Error Resume Next" would be nice.

Consider the following constructor:

   1:          public ConnectionBase(string server, int port)
   2:          {
   3:              m_server = server;
   4:              m_port = port;
   5:              try
   6:              {
   7:                  m_ipep = new IPEndPoint(Dns.GetHostEntry(server).AddressList[0], port);
   8:              }
   9:              catch (SocketException) { }
  10:          }

This class essentially encapsulates a TcpClient.  Now, what I do here in the constructor is attempt to resolve the host address.  In the event that there is an error, for whatever reason, Dns.GetHostEntry will raise a SocketException, typically telling me that the URI could not be found.

Now, the general wisdom about SEH is to not just "eat" an exception; rather, we prefer to do something worthwhile with it.  But in this case, I haven't invalidated the state of my class, I haven't invalidated the state of the system, or done anything to harm my class.  In fact, if the m_ipep (an IPEndPoint object) isn't initialized when Connect() is called, it attempts to do it as well:

   1:          public virtual bool Connect()
   2:          {
   3:              // ...
   4:              if (m_ipep == null)
   5:              {
   6:                  try
   7:                  {
   8:                      m_ipep = new IPEndPoint(Dns.GetHostEntry(m_server).AddressList[0], m_port);
   9:                  }
  10:                  catch (SocketException se)
  11:                  {
  12:                      OnError(string.Format("Your computer was unable to resolve hostname {0}.  If necessary, add an entry to %SystemRoot%\\system32\\drivers\\etc\\hosts, or flush your DNS resolver cache, and try again.",
  13:                          m_server), se);
  14:                      return false;
  15:                  }
  16:              }
  17:              //...
  18:              return true;
  19:          }

This method correctly bubbles an error to the using classes in the event that the IP end point could not be located.  You might ask, though - what is the advantage of using early resolution?  That is, I could drop the exception code altogether.

Well, if someone is debugging their code using my library and has first-chance exception handling enabled, it is easy enough to spot where a parameter might be causing issues.

Once the class is initialized, someone using a debugger can see the internals of my class, including the resolved end point.  This may help them identify the remote end point's IP address; perhaps that particular server is not functional.

There are other issues with the "best practices" as well -- for instance, they are sometimes at odds with each other.  The .NET Framework Guidelines say not to overuse catch, but also to not to just catch a general exception.  Consider:

   1:              try
   2:              {
   3:                  // create an instance of this variable to make sure that the data provider can be loaded.
   4:                  DataProvider provider = DataProvider.Instance;
   5:              }
   6:              catch (TypeLoadException tle)
   7:              {
   8:                  throw new ConfigurationErrorsException(
   9:                      string.Format("Type '{0}' was not a valid type according to the runtime.", keyProviderType), tle);
  10:              }
  11:              catch (TargetInvocationException tie)
  12:              {
  13:                  throw new ConfigurationErrorsException(
  14:                      string.Format("An exception was raised in the constructor of type '{0}'.  More information may be available in the contained exception.", keyProviderType), tie.InnerException);
  15:              }
  16:              catch (MethodAccessException mae)
  17:              {
  18:                  throw new ConfigurationErrorsException(
  19:                      string.Format("Security policy prevented access to the default public constructor of type '{0}'.", keyProviderType), mae);
  20:              }
  21:              catch (MissingMethodException mme)
  22:              {
  23:                  throw new ConfigurationErrorsException(
  24:                      string.Format("No default public constructor existed on the type '{0}'.  Change the application configuration so that the 'dataProvider' property of the 'dataConfiguration' section points to a type with a default public constructor and inherits from the '{1}' type.", keyProviderType, typeof(KeyProvider)), mme);
  25:              }
  26:              catch (MemberAccessException memae)
  27:              {
  28:                  throw new ConfigurationErrorsException(
  29:                      string.Format("Could not create an instance of the abstract type '{0}'.  Change the application configuration so that the 'dataProvider' property of the 'dataConfiguration' section does not indicate an abstract type.", keyProviderType), memae);
  30:              }

In the code above, I have legitimate reasons for catching these - retrieving the DataProvider.Instance property is documented to raise these exceptions, which are in turn raised by Activator.CreateInstance.  The real problem with this kind of situation is that the outer calling code needs to be shielded, and so ultimately, the calling code handles only a ConfigurationErrorsException.  This in turn is handled and logged to the event log before terminating the Windows Service that hosts this code.

You can read more in the .NET Developer's Guide - Exception Handling article on MSDN.  Just bear in mind - you probably want different exception handling strategies if you're writing a user interface vs. library code vs. data access code.  At the end of the day, it's how much bad stuff you want to show your users, how much bad stuff you can handle internally to your application, and how much bad stuff you're willing to let your application produce because you over-caught exceptions.

14Feb/080

Comment Responses: C# 3.0 Best Practices

Posted by Rob

I've received some comments and composed some responses, included below:

First Comment/Question:

I've got questions about performance in LINQ and LINQ to SQL. 
Is it more effective to create a query against the context or against a collection on another object?  For example, which is better:

     = from t in db.Things
        where t.Something = "MyVal"
        && t.ForeignKeyId = 28

or is it better to do this:

     = from t in db.ForeignKeys
        where t.Something = "MyVal"

?  In the former, I'm running against the data context, in the latter, the (kinda) array of foreign key matches

Is it more effective to select the object or a property in the object if I am using an agregate function?  For example:

    = ( from t in db.Things select t ).Count()

or is it better to do this:

    = ( from t in db.Things select t.ThingId ).Count()

?  In the former, I select the entire t but do nothing.  Does it actually query the data, or just a place-holder?  In the latter, I've got a specific property, which I still don't need.

My answer:

I'm going to preface this by saying - as far as LINQ goes, there's SO MUCH in the toolset, and I can't claim to be much more than a novice.  What I can say about it, though, is that I can make some educated guesses about how LINQ will behave vs. LINQ-to-SQL.

In your first example, with the query against the database, looking at the "where t.Something == "MyVal" && t.ForeignKeyId == 28" (you're using double-equals-signs in those, right? ;-)), you're probably better-off doing the complete query inside of the where clause.  Especially in LINQ-to-SQL, where that where clause is going to end up in a SQL statement anyway, you're going to be pulling against a database column, which should be fairly fast.  The only performance tweak I could suggest in this instance is ordering the foreign key ID comparison first, to avoid the char-by-char comparison of a string compare if the foreign key IDs don't match up.  This is kind of a difficult choice to make, as it might sacrifice a magnitude of code readability ("Why is Rob checking a group of values for their foreign key ID?"). 

In your second question related to the Count() aggregate, your LINQ-to-SQL and LINQ-to-Objects queries will (I believe) have similar performance characteristics, but for different reasons.

If you're using LINQ-to-SQL, the entire query should be ported to SQL and should end up reading like:

SELECT COUNT(*) FROM db.Things;

The alternative (in the alternative case you suggested) would be:

SELECT COUNT(ThingId) FROM db.Things;

I don't recall if COUNT counts NULL rows, and so you have a potential for actual incorrect data if you use the latter, though if you're sure to go against something like the PK column you're probably allright.

In any case, I don't believe it really matters.  When using LINQ-to-SQL, you'll pull from the DB.  I just pulled up a SQL Profiler, ran a query from LINQ, and I got the results I expected:

SELECT COUNT(*) AS [value] FROM [Events].[Events] AS [t0]

So it looks like we're good.  Unfortunately this query can't be visualized like non-aggregate queries:

var result = (from ev in context.Events select ev).Count();

Hovering over "result" just results in a 0 being displayed.

As for LINQ-to-Objects, my personal opinion is to use the first syntax (select t) as opposed to creating a new anonymous type.  The reason for this is that a new series of objects needs to be created, even if it just contains your one property. 

One of the things we're doing in 3.5 is to extend our existing object model, which generally shields us from database-specific implementation code, to support objects like data contexts.  However, any code generated by LINQ-to-SQL will ONLY be in our provider-specific implementation libraries that are dynamically bound at runtime.  This allows us to program with LINQ-to-SQL when we want to, but it requires that we map .NET Entity objects to our custom entity objects.  Still, this is sometimes beneficial.

Second Comment/Question:

I don't understand this discussion on Object Initializers:

"Always validate input to properties; an exception raised while setting a property will cause a memory leak, even inside try/catch

The object will already be allocated if an exception is raised within a property"

I am not sure how to express what it is that I do not understand, so maybe you can just talk to this a little bit. Typically, I would not validate parameters in an object initializer scenerio (unless they came from an external source, of course, such as user input). For example, if I pull data out of a database (my database) and build an object from it, I would not validate the data first - an exception seems correct in this case (corrupt database data). I also would not fear "memory leaks" because I assume that my "partially-constructed" object would be garbage collected like any properly-constructed object would.

In other words, I image that using object initializers is *equivalent* to (a) creating an object using the default contructor, followed by (b) setting properties. If an exception occurred setting properties - so what? [At least from "memory leak" point of view.]

My answer:

If you're populating from a database, your assumption is that the input is going to be valid, and that's certainly a valid point.  That you're thinking about this is really what I was after.

The "memory leak" I'm referring to is, again, a potential non-issue as you pointed out (the stranded object will indeed get garbage-collected).  The problem is, for me anyway, that we have no idea and no control over *when* that object gets garbage-collected, or how.  In fact, because it's implicitly compiler-created, we have no way to do anything at all with that temporary object.

Suppose we had this code:

   1:  SomeObj blah;
   2:  try {
   3:      blah = new SomeObj() { ExceptionTest = "This text generates an exception." };
   4:  } catch { }

Even here, the "<>blah_temp01" object (or whatever it's called) is still valid on the stack, but we have no way to access it.  The benefit is that code still behaves as if it was part of a constructor, and the "blah" variable is null (which is my guess of what the developers were gunning for).  But until we run out of memory and do a GC, that memory's still there, still allocated, and possibly causing heap fragmentation.

The other difficulty we run into, and this is perhaps a cheap excuse, is that when all of the properties are set on the same line, we can't tell (at least in Visual Studio) which line is causing the exception.  We could do this:

   1:  blah = new SomeObj() {
   2:                          ExceptionTest = "This text generates an exception."
   3:                       };

But does that really gain you much over:

   1:  blah = new SomeObj();
   2:  blah.ExceptionTest = "This text generates an exception.";

Ultimately, my point is this: you can handle cleaning up, disposing, and you otherwise know that an object exists when you don't use initializers.  You don't when you don't use initializers.  Suppose I have a smart card file stream that implements IDisposable.

Stream fs = new SmartCardFileStream() { FilePath = "003F/0040", Mode = FileMode.ReadWrite };

Suppose that, for whichever reason, the file opens when you set FilePath.  But, FileMode.ReadWrite isn't supported (only read or write).  You can't programmatically close that file; you have to wait for the finalizer to be invoked.  Arguably, you should wrap that declaration within a using {} block, but that's not always feasible (e.g., when you need to deal with it across events in a Winforms app). 

The primary difference is that, when an exception is raised in a constructor, it is automatically marked for garbage collection by the CLR (except in cases of a static constructor raising a TypeInitializationException, making the whole class inaccessible for the duration).  A partially-"constructed" class that raised an exception during a property-set would be problematic in that it doesn't get that benefit - we have to rely on the garbage collector to determine that there are no outstanding references to it at the next GC pass.

Summary

I chopped off some of the thank yous/hellos/I appreciates from the e-mails, and just wanted to once again say that I appreciate the comments and feedback.  Hopefully, those of you who chose to comment weren't too thrown off by my e-mail replies; I'm not particularly a fan of starting threads and posting comments on my own blog post (although I may once I get a new blog).  Also - I'd just like to point out that these are my personal opinions and not necessarily the end-all, be-all of C#; there are certainly places to use each new feature, and my goal is to investigate how features are implemented so that we know when a situation calls for them.

12Feb/080

C# 3.0 Best Practices: Downloads

Posted by Rob

I'm publishing the PowerPoint slides and sample code from the C# 3.0 Best Practices talk I gave tonight at the Arizona .NET Users Group meeting.  Once again I'd like to thank Scott Cate for extending the opportunity to me, Lorin Thwaits for pinch-hitting for Scott, and to Hudson IT Staffing for the event sponsorship.

Please note that the code sample isn't really annotated, and was designed to generate the code in the slides and the Reflector output shown.  The theme of the presentation wasn't so much "How can we do these things the best?" but more "How is this implemented, so I can choose the best way to do something?"  So the code isn't by any means a beacon of effectiveness - it's meant to go with the slides.

I'd welcome feedback and comments, as well as suggestions for additional content.  Enjoy!

3Feb/080

Looking at .NET: The Disposable Pattern

Posted by Rob

One of the more obscure things about the .NET Framework is the Disposable pattern used throughout the framework, supported via the IDisposable interface.  This pattern is so pervasive throughout .NET, that C# intrinsically supports it via the using keyword.  There is also a standard pattern for implementing the interface that the interface just can't express (perhaps because interfaces can't specify protected methods; maybe that's a C# 4.0 Wishlist part 6 item?).

We can use an IDisposable object with the using keyword like so:

   1:  using (SqlConnection con = new SqlConnection(WebConfigurationManager.ConnectionStrings["DbConnection"].ConnectionString))
   2:  using (SqlCommand cmd = new SqlCommand("[dbo].[GetAllItems]", con))
   3:  {
   4:      cmd.CommandType = CommandType.StoredProcedure;
   5:   
   6:      con.Open();
   7:      using (SqlDataReader reader = cmd.ExecuteReader())
   8:      {
   9:          return GetAllItemsFromReader(reader);
  10:      }
  11:  }

In this example, SqlConnection, SqlCommand, and SqlDataReader all implement IDisposable, because they interoperate with unmanaged code.  By using the using blocks, the C# compiler actually transforms these into try/catch blocks:

   1:  List<Item> _compilerGeneratedResult;
   2:  SqlConnection con = null;
   3:  SqlCommand cmd = null;
   4:  try 
   5:  {
   6:      con = new SqlConnection(WebConfigurationManager.ConnectionStrings["DbConnection"].ConnectionString;
   7:      cmd = new SqlCommand("[dbo].[GetAllItems]", con);
   8:   
   9:      cmd.CommandType = CommandType.StoredProcedure;
  10:   
  11:      con.Open();
  12:   
  13:      SqlDataReader reader = null;
  14:      try
  15:      {
  16:          reader = cmd.ExecuteReader();
  17:          _compilerGeneratedResult = GetAllItemsFromReader(reader);
  18:      }
  19:      finally
  20:      {
  21:          reader.Dispose();
  22:      }
  23:  }
  24:  finally
  25:  {
  26:      if (cmd != null)
  27:          cmd.Dispose();
  28:      if (con != null)
  29:          con.Dispose();
  30:  }
  31:   
  32:  return _compilerGeneratedResult;

Yeah, if it was left up to the C# users to use this pattern correctly, we'd never do it (not without the using keyword, anyway).  But the question is, how do we implement it?

Traditionally, we implement it by creating a protected, virtual method (or private method if the class is sealed), that accepts a boolean value that indicates whether we're calling this method via Dispose() or via the destructor.  The Dispose() method calls this with true, and we also implement a destructor that calls it with false.  The Dispose(bool) method then implements the cleanup logic, and if the parameter is true, also tells the garbage collector to not perform the invoke the finalizer (the destructor) on this object.  Here's a sample:

   1:  using System;
   2:  using System.Runtime.InteropServices;
   3:   
   4:  namespace DisposableSample
   5:  {
   6:      public class HGlobalPtr : IDisposable
   7:      {
   8:          #region IDisposable Members
   9:   
  10:          ~HGlobalPtr()
  11:          {
  12:              Dispose(false);
  13:          }
  14:   
  15:          public void Dispose()
  16:          {
  17:              Dispose(true);
  18:          }
  19:   
  20:          protected virtual void Dispose(bool disposing)
  21:          {
  22:              if (disposing)
  23:                  GC.SuppressFinalize(this);
  24:          }
  25:   
  26:          #endregion
  27:      }
  28:  }

This is the most basic implementation of the IDisposable pattern.  We're going to evolve it a bit, actually add use to it, and also look at Static Code Analysis (SCA) output from this.  First, to the SCA (using FxCop).

This basic example generates two warnings, incidentally, from the same rule.  Here they are:

warning : CA1816 : Microsoft.Usage : Change 'HGlobalPtr.Dispose()' to call 'GC.SuppressFinalize(object)'. This will prevent unnecessary finalization of the object once it has been disposed and it has fallen out of scope.

warning : CA1816 : Microsoft.Usage : 'HGlobalPtr.Dispose(bool)' calls 'GC.SuppressFinalize(object)', a method that is typically only called within an implementation of 'IDisposable.Dispose'. Refer to the IDisposable pattern for more information.

If you look at help for this rule, you'll see that to properly implement this change, you actually call GC.SuppressFinalize(this) within the Dispose() method (as opposed to Dispose(bool) method).  Doing so ensures that GC.SuppressFinalize is called via IDisposable.Dispose even if derived classes override the virtual Dispose(bool) method.

Here's a more complete implementation of HGlobalPtr, with IDisposable fully implemented:

   1:      public class HGlobalPtr : IDisposable
   2:      {
   3:          private IntPtr m_ptr;
   4:   
   5:          public HGlobalPtr(int size)
   6:          {
   7:              m_ptr = Marshal.AllocHGlobal(size);
   8:          }
   9:   
  10:          #region IDisposable Members
  11:   
  12:          ~HGlobalPtr()
  13:          {
  14:              Dispose(false);
  15:          }
  16:   
  17:          public void Dispose()
  18:          {
  19:              Dispose(true);
  20:              GC.SuppressFinalize(this);
  21:          }
  22:   
  23:          protected virtual void Dispose(bool disposing)
  24:          {
  25:              if (disposing)
  26:              {
  27:                  // free the state of any contained objects
  28:                  // we don't contain any other objects!
  29:              }
  30:              // free my own state
  31:              if (m_ptr != IntPtr.Zero)
  32:              {
  33:                  Marshal.FreeHGlobal(m_ptr);
  34:                  m_ptr = IntPtr.Zero;
  35:              }
  36:          }
  37:   
  38:          #endregion
  39:      }

Now, there are other code analysis problems; I need to handle security warnings for calls to Marshal.AllocHGlobal and Marshal.FreeHGlobal, which have a LinkDemand permission set on them.  I should also consider replacing m_ptr with a SafeHandle.  But aside from these, IDisposable is correctly implemented.

Just remember - if your object is IDisposable - please, use the using () statement!

For more information on implementing IDisposable, refer to the Microsoft documentation article on Technet.