Tuesday, September 17, 2013

Using ReportViewer in ASP.NET MVC

About three and a half years ago I was tasked with building a reporting portal. I elected to go with ASP.NET MVC due to the fact that most of the rest of my department was a Ruby shop. I figured if anyone else would have to come in and work on it, at least they wouldn't have to struggle with the idea of ASP.NET WebForms.

Now one challenge was that this was a publicly facing site, but the reports could only be seen by authenticated users with the right permissions. The main challenge however is the fact that ReportViewer requires that a page be able to store Session State. By definition MVC is Stateless.

So how do we solve this problem? Luckily the folks at Microsoft allowed us to have WebForms and MVC applications side by side. So the simplest solution is to just include a new webform page as part of the Solution.

The first thing we need to do is get our criteria. I'm assuming that you want to pass in some parameters to the ReportViewer, so lets grab those from our Form Collection and build our Action.

[AcceptVerbs(HttpVerbs.Post)]
public ActionResult BuildReport(FormCollection collection)
{
       try
      {
            // Load Form Data into a report object
            DynamicReport report = new DynamicReport( collection["AccountName"], Convert.ToDateTime(collection["StartDate"]), Convert.ToDateTime(collection["EndDate"]));
           // Save Report to Session
           Session["Report"] = report;
           //Redirect to Reports WebForm Page
            Response.Redirect("../Report.aspx");
        }
        catch (Exception e)
        {
                  return RedirectToAction("Error", new { id = (int)ErrorCode.Unknown });
        }
}

Its just that easy. Can't believe this has been sitting in draft status for that long....
          

Monday, September 16, 2013

Software Development and Quality Engineering

I can't tell you how many times I have heard developers and managers pay lip service to quality and testing. I cringe every time I hear the words "quality is important to us, but we need this done ASAP!" from management. I likewise cringe every time I hear a developer say "I did a quick test on it, QA will catch anything I missed". Both of the previous statements are just wrong. More importantly they are idiotic and a total fucking cop out. If code quality is of any importance whatsoever, it is absolutely VITAL that you think about the quality of the final product before you even start coding.

There are two huge hurdles that most organizations must overcome if they want to produce a quality product. These two hurdles are going to sound so idiotically simple, that you may stop reading, but please bare with me, as I explain. One hurdle is time, the second is terminology. I know those sound kind of silly but hear me out.

Time

We all know that there are deadlines that must be met. Sometimes we absolutely positively MUST get a deliverable out the door. The thing is, this does not happen in a vacuum. Working more hours, eating into testing time, sacrificing writing unit tests, throwing in hacks that will be "fixed later" all ends up sacrificing code quality. 

I'm sure as developers we've all at least heard of the mythical man month. The idea that throwing more developers at a project actually makes the project last longer. Yet this same phenomenon seems to be completely ignored when it comes time to complete Quality or User Testing! You cannot compress testing time and throw more testers at a problem and expect a quality result! It doesn't work that way! Have you ever seen a large group of testers testing functionality? It's a logistical fucking nightmare to begin with. Duplicate bugs are regularly reported leading to multiple developers trying to fix the same code from multiple different spots, which either leads to wasted time and duplicate effort and frustrations by both developers and testers!

Yet it seems in many places when a testers say we need two weeks to test some critical part of the system, it's viewed as a nice to have. This is simply not the case! Good testers understand that what they are doing is making sure that user doesn't have a "what the fuck" moment. Ultimately I hope you realize what a user with too many "what the fuck" moments can also be a called an "ex-user".

Terminology

This brings me to my next point which is terminology. I purposely used the word "tester" in my previous section, but you know what, that's the wrong word. In Software Engineering we are so fortunate to have a vocabulary that allows us to express abstract concepts to quickly convey meaning. Language, Object Oriented, Classes, Methods, Interfaces, Factories, Constructors, Compiler, Scripts all great words, and for the most part universal for developers. These words are important in their own ways, and yet it seems for some reason when it comes to code quality and testing we don't have quite the same vocabulary.

I absolutely abhor the word "Tester" to describe what these individuals do. Oh sure, they run tests, but that just makes it sound like something a monkey could do, and quite frankly that's just very, very wrong. Likewise I dislike the term Quality Assurance as that term seems to express the idea that quality is something that should be bolted on to the end of a project. Which in turn leads to the perception that you are "testing" the deliverable. This too is wrong.

Quality should be the focus throughout the process. Quality of the code written. Quality of the design and architecture. Quality of the final deliverable. This is not an process that can happen at the end, but a process that should happen THROUGHOUT the cycle! So since this is something that should happen throughout the process, the job description and name should change. I personally like the term that my friend and boss Pat Richards uses: Quality Engineering. After all that is what this process is aiming to do! 

Most importantly though, by changing the name to Quality Engineering you change what it is you are actually trying to accomplish. You are engineering quality right into the process. It makes much more sense to have someone who is called a Quality Engineer being a part of the initial design and estimation sessions, and that is a huge part of why Terminology is so important! 

Changing what you call something can totally revamp the way you look at its function. Equally important is making sure that everyone uses the SAME terminology. I have had a developer say to me "I am in the process of unit testing my code." That to me meant they were writing automated unit tests, since that's the terminology I use. What they meant was that they were doing developer testing on the module they had just wrote. They weren't wrong with how they phrased it, but because we weren't on the same page with terminology, I made some very wrong assumptions on the code base.

The same problems can arise on the transition from Development to Testing cycles. This too is why it is so important that Quality Engineers be involved in the whole process from beginning to end. By being embedded with the team that is developing a module, they come to use the same common terminology as the team, thereby decreasing communication problems and increasing understanding and thereby productivity. After all not having to circle back to make sure you think you understand whats going on is a huge time saver. 

 So bottom line is if you want to make sure you are delivering quality products, you have to be sure that you have a process in place that supports it. Robbing Peter to pay Paul by giving developers more time to write code and sacrificing final testing time is a quick way to failure. Likewise treating testing like it is something that CAN be sliced and sacrificed is a quick way to doom you deliverable to failure (or at least poor results). Never underestimate the importance of your choice in wording, and naming. Names have power that create preconceived notions in others. If you want to ensure quality, make sure the terminology that you use empowers that agenda. In addition make sure everyone is on the same page and speaking the same language. Anything short of that is a quick road to a bad result.

Friday, September 13, 2013

Coding for readability

I've read many books, sites and blog posts that explore the topic of what it means to write "good code". As a general rule they seem to mostly come down to "write the most efficient code possible". While as a rule of thumb I feel you should keep an eye on efficiency, I would argue that a more important point is writing code for readability. 

The reality is that over time even the most inefficiently written code will run better on faster hardware. Horribly inefficient code written 10 years ago runs blazingly fast today. That's not to say that you should sacrifice efficiency because it doesn't matter, but if you sacrifice readability for efficiency you end up causing problems later on. Let me give you an example.

    public class HelperClass
    {
        public HelperClass ()
        {
        }

        public void All(int x, int y)
        {
            for (int i =x; i<=y; i++) 
                Console.WriteLine (i);
        }

        public void Evens(int x, int y)
        {
            for (int i =x; i<=y; i++) 
                
//if its modulus is 0 it must be even
                if (i % 2 == 0) 
                    Console.WriteLine (i);
        }

        public void Odds(int x, int y{
            for (int i =x; i<=y; i++) 
            
//if its modulus is not 0 it must be odd
            if (i % 2 != 0) 
            Console.WriteLine (i);
        }

        public static void DoIt(int x, int y)
        {
            
HelperClass hlpr = new HelperClass();
            hlpr.All(x, y);
            hlpr.Evens(x, y);
            hlpr.Odds(x, y);
        }
   }


Look its a simple example, I know. I'm not trying to show off some elite coding skill, that's not the point. I want this to be useful to as many people as possible. So let me bring up a few things that could seriously increase readability.

Naming Your Classes

public class HelperClass

The word helper is REALLY tempting to use. After all, if you are coding very quickly (and lets be honest with deadlines we all take shortcuts), it can be tempting to just say "I'm going to be using this code in more than one place, because I'm trying to write good DRY (Don't Repeat Yourself) code". This leads to two problems though. The first is that when you go to actually call your code you end up with calls like this:

    var hlpr = new HelperClass();
    hlpr.All(x, y);

Ugly. Taken out of context, what the fuck is this actually doing? When you are trying to rip through this code in three weeks time trying to debug something, you are going to waste time circling back and reminding yourself what it is you were trying to write. The Second problem it brings up is that when you name something helper, it almost IMMEDIATELY becomes a dumping ground. It's amazing what ends up in those classes after a team of developers has had one of those around for a while. After a while they become a huge amorphous blob of crap and cruft that has to then has to be refactored thereby changing HUGE underpinnings of your application with it. It's not pretty I don't recommend it. So whats the easiest way to solve this? Call it what it is! In this case, public class NumberPrinter!

    NumberPrinter prtr = new NumberPrinter();
    prtr.All(x, y);

That's better but I think this leads to my next point

Naming Your Variables

    NumberPrinter prtr = new NumberPrinter();
    prtr.All(x, y);

Guys, you really aren't saving anything other than keystrokes if you don't give good descriptive variable names. The compiler doesn't care what you call your variable, but you know who will? The next developer who comes along. Changing this variable name to numberPrinter goes along way to making people happy.

    NumberPrinter numberPrinter = new NumberPrinter();
    
numberPrinter.All(x, y);

Well that's good, but now look at the line numberPrinter.All(x, y). I mean that doesn't exactly read well. Plus going back to my previous point of dumping grounds, the word All in this case is a little generic...

Naming Your Methods

        public void All(int x, int y)
        {
            for (int i =x; i<y; i++) 
                Console.WriteLine (i);
        }


What is this method actually doing? All is not very descriptive. How about giving it a more accurate name, like PrintAllNumbersBetweenBeginningNumberAndEndNumber? Sure its a lot longer but lets look at it in use:

    public void PrintAllNumbersBetweenBeginningNumberAndEndNumber(int x, int y)
    {
        for (int i =x; i<y; i++) 
            Console.WriteLine (i);
    }

That parts not too bad.

    NumberPrinter numberPrinter = new NumberPrinter();
    
numberPrinter.PrintAllNumbersBetweenBeginningNumberAndEndNumber(x, y);

Holy crap! I now know what that is doing without looking at the code! YAY! But wait, which parameter is the beginning and which one is the end... I damn, guess that needs fixing too!

Naming Your Parameters

        public void PrintAllOddBetweenBeginningNumberAndEndNumber(int x, int y)
        {
            for (int i =x; i<=y; i++) 
                
//if its modulus is not 0 it must be odd
                if (i % 2 != 0) 
                    Console.WriteLine (i);
        }

Anyone want to guess what this method is doing? Oh wait, you don't have to guess! You can tell from the name! How awesome is that! Unfortunately we don't know which parameter is the beginning and which is the end. Naming a parameter follows the same idea as both variables methods and classes! Be descriptive! Lets label our parameters with what they actually are to the method! So x becomes beginningNumber, and y becomes endingNumber! Brilliant!

    public void PrintAllOddBetweenBeginningNumberAndEndNumber(int beginningNumber, 
                                                          int endingNumber)
    {
        for (int i = beginningNumber; <= endingNumber; i++) 
            //if its modulus is not 0 it must be odd
            if (i % 2 != 0) 
                Console.WriteLine (i);
    }

Well that IS better. I mean look:

NumberPrinter numberPrinter = new NumberPrinter();numberPrinter.PrintAllNumbersBetweenBeginningNumberAndEndNumber(beginningNumber, 
                                                                endingNumber);

But Alex, you are thinking what about the i variable in the for loop! That's not descriptive. There is so many ways you can describe the counter in a for loop. counter being one of them. If you want to get creative that's fine, but most people learn this structure using i, so generally this is a pretty safe use for a crappy variable name. But you know whats bothering me about the method code? Lack of curly braces...

Use White space, and available Lexemes!

What to I mean by Lexeme? Well in this case I'm going to referring to Curly braces! We all know that in C languages you don't HAVE to use them all the time, there are exceptions. But in this case I think we may want to look and make sure we are making our code readable:

public void PrintAllOddBetweenBeginningNumberAndEndNumber(int beginningNumber, 
                                                          int endingNumber) {
            for (int i =beginningNumber; i<=endingNumber; i++) 
            
//if its modulus is not 0 it must be odd
            if (i % 2 != 0) 
            Console.WriteLine (i);
        }


Look Ma! I moved my white space and curly braces wherever I felt like! Guess what the computer can read that correctly! Other developers, will probably fuck up, and then get pissed off when can't figure out why there code isn't working. Just because you don't HAVE to use something, doesn't mean you shouldn't. I don't HAVE to use the turning signal on my car, but generally I feel its a pretty good idea. It makes it a bunch easier for other drivers on the road to know what it is I am actually trying to do... So lets fix this!

public void PrintAllOddBetweenBeginningNumberAndEndNumber(int beginningNumber, 
                                                          int endingNumber) {
            for (int i =beginningNumber; i<=endingNumber; i++) 
            {                //if its modulus is not 0 it must be odd
                if (i % 2 != 0)
                { 
                    Console.WriteLine (i);
                }
             }        }

Well that's better! But does anyone see an issue here? Were using two different styles for our curly braces! That's a recipe for future WTFs. 

Be Consistent

Use your lexemes in a consistent manner. If you want to put curly braces at the end of your if statement, use that for all statements! If you work with other developers make sure you ALL agree on a usage. Having differences in White spacing and statement terminators is a quick way to create a debugging nightmare.



Code Comments

That's right I put a code comment in there! Generally I'm not a huge fan of code comments. I think to many people rely on them for really poorly written code. If you write more comments than code, then quite frankly, you are doing it wrong. Now I realize in this case I put a comment over a line with modulus. Honestly that was the simplest thing I could think of that people may not recognize. I've met a few developers who have never come across the modulus operator, it happens. So keeping it short, I am basically telling the next developer "If you don't know what the % symbol means, it means modulus, and if you don't know what that is, google it!"

So Where did we end up?


public class NumberPrinter
{
    public NumberPrinter ()
    {
    }
 
    public void PrintAllNumbersBetweenBeginningNumberAndEndNumber(
                                                            int beginningNumber, 
                                                            int endingNumber)
    {
        for (int i =beginningNumber; i<=endingNumber; i++) 
        {
            Console.WriteLine (i);
        }
    }

    public void PrintAllEvenNumbersBetweenBeginningNumberAndEndNumber(
                                                            int beginningNumber, 
                                                            int endingNumber)
    {
        for (int i =beginningNumber; i<=endingNumber; i++) 
        {
           //if its modulus is 0 it must be even
           if (i % 2 == 0) 
           {
               Console.WriteLine (i);
           }
        }
    }

    public void PrintAllOddBetweenBeginningNumberAndEndNumber(
                                                            int beginningNumber, 
                                                            int endingNumber)
    {
        for (int i =beginningNumber; i<=endingNumber; i++) 
        {
            //if its modulus is not 0 it must be odd
            if (i % 2 != 0) 
            {
                Console.WriteLine (i);
            }
        }
    }

    public static void PrintAllNumbersEvenNumbersAndOddNumbersBetweenBeginningNumberAndEndingNumber(
                                                            int beginningNumber, 
                                                            int endingNumber)
    {
        NumberPrinter numberPrinter = new NumberPrinter();
        numberPrinter.All(beginningNumber, y);
        numberPrinter.Evens(beginningNumber, y);
        numberPrinter.Odds(beginningNumber, endingNumber);
    }
}

Well that's certainly more human readable, and you know what we didn't do a damn thing to the efficiency! Win! It's as simple as this, if you use a compiler, then any code you write can be read by a computer. Honestly that's the easy part. Writing code that future developers can read, understand and extend? That's hard. So many times we get lost in the sea of efficiency of our code from the machines perspective, but we need to make sure we don't lose sight of the efficiency of bring a new hire up to speed. Or reading through code that hasn't been touched in a long time because we need to update or extend its functionality. Or god forbid even having to dig into the deep dark reaches of the code for a heisenbug! The bottom line is don't neglect the human aspect of efficiency. After all, humans don't get faster hardware every few years.