Sohan's Blog

Things I'm Learning

How Not to Provide Feedback When Doing Code Review

Code review, as any other review process, can often be an attack on someone’s ego and incite anger and frustration. Being both on sending and receiving end of such code reviews for years, I have learned a few how-not-to-do-it, or as one might say, anti-patterns of providing code review feedback. Let me know if you agree/disagree with me on the following:
  • You are always reviewing Adam’s code while Adam is never reviewing yours.
  • Your feedback is like “Adam, you should break down part of this method into a private method” instead of ”we can probably extract a part of…”
  • You are always suggesting, for example ”we should do x and y and z”, instead of asking interesting questions ”can we do x? what if we did y?”

Service API and Exceptions


Too often I see I am using a REST/SOAP API to talk to a third party system that frustrates me because of poor error messaging. Here’s an example to illustrate a typical frustration:

Call: city_service.update_residence_address(city_dueler, new_address)
Response: <status>Failed</status><error>Invalid address</error>

But the address just seems right to me. So, I need to know specific reason about why the address is wrong. The error message leaves this critical detail. What happens next is, I look for the log files created by the city_service, conceptually supposed to be a third party hosted service. To do this, I SSH to the server, then cd to the logs folder and get lost in many of the cryptic log files. Then eventually I find a log and if I am lucky, I find something like the following:

InvalidAddressException at:…

Street number is not listed..


Now, I know I had an issue with the streer number. But I can see this only after I looked into the log of the thirdparty server. In an ideal case, I don’t have an access to their server. Even if I have it, I cannot easily understand their logs and stack traces! In most API calls to third party applications I have had this frustration to varying degrees.

Here’s a lesson learned:
Next time you expose an API for another app, expose error messages as specific as possible - so that, no details is left buried into your log files.

NuGet - Why Should You Care?

Traditionally the .NET community, or more appropriately the users of .NET framework relied on products from Microsoft - so, in a product stack you are likely to see almost everything coming from Microsoft. However, some community contribution made its way into the main stream - for example, NUnit or Log4Net. But you can literally count the number of such main stream non-Microsoft products in your development stacks with your fingers (sparing a few)!
NuGet makes it easy to share your reusable library/utility code to the community. So, if you’ve done something cool that you wanna share, you should utilize the platform.
One important difference between your and Microsoft’s contribution is, whatever you are contributing is likely to be a piece of software extracted from one of your real world projects. On the other hand, Microsoft attempts to produce something based on public interest and their imagination - often missing the real pain that you or I have. So, this mismatch between the designer and the actual consumer of the product often leaves a lot of opportunity for you. If you did something to solve your own pain on a project, its likely you’re not alone. So, share it with us.
NuGet official feed and website is a great place to get feedback.
Did something cool? Well, you should feel good as people will use and appreciate your work. More importantly, they will provide you with interesting ideas and reviews that you haven’t thought about. Does this sound useful to you? People will go even further - they will directly contribute to the project with code!
NuGet will challenge you with competition from other contributors.
A healthy competition is a great way of learning from others too. You’ll see other contributors attacking the same pain you are solving in a different approach, often directly challenging you! You love challenge, don’t you?
If you need hard numbers to get motivated, here’s some data from my MvcMailer project:
  • Downloaded 600+ times in less than 2 months.
  • Received 60+ emails from people using MvcMailer, mostly encouraging feedback.
  • 300%+ increase in my blog traffic.

Bottom line is, you should publish your NuGet package if there’s any cool project you have done. Or, look out for what others are doing and possibly contribute with your code/suggestions. If you need an idea, look for StackOverflow questions, you’ll see there are solvable problems that people are fighting against time and again.

My 2011 Q1 Developer Roadmap

For me, 2011 Q1 has so far been a good exposure to new techniques, tools, articles and books, thanks to ThoughtWorks book allowance! However, if you are interested, here’s what’s keeping me busy:
  1. User Experience: I find a good user experience is THE thing that you want in anything you design, and if you are writing a software, its even more important. After I read a few books, I have a feeling that its not just a common sense approach, it takes some education and a deep care to produce something usable. Check out my reading list at http://smsohan.com
  2. HTML5: HTML5 is so more than just knowing How To Meet Ladies (HTML)! The popular browsers are all way ahead of the official HTML5 release date, so if you are building something on the web, its time you give it a real shot. Again, I shared my reading list on my website.
  3. .NET NuGet: Microsoft .NET just got a major component, NuGet package manager, that greatly simplifies the sharing and consuming of reusable libraries. Already there are tens of thousands of downloads for the popular .NET libraries and I believe its just a start. So, if you’ve done something cool in your project and want that to share with the .NET crowd, you should consider NuGet as a distribution channel and get some traction on it.
  4. MvcMailer: MvcMailer has got all my developer attention in the recent past. If you haven’t checked already, I highly encourage you take a look at its features at this page and you’ll see its a full stack elegant emailing solution that you need in your ASP.NET MVC projects. However, while working on this, I got familiar with the internals of the ASP.NET MVC source code and I consider this as a good experience.
  5. Functional Programming: Did you learn one? Which one? Please suggest me your favorite functional language. In the remaining of this Q1, I want to spend some time on a functional programming language, probably F#.
Its inspiring to see Calgary getting warmer these days - hope we get some outdoor soccer by the end of 2011 Q1, that would be awesome!

Comments

Sohan
Thanks for sharing the link. I started reading the online book at http://www.ctocorner.com/fsharp/book/
I like the approach in this book as it goes easy on the functional stuff for someone like me whose brain is prefilled will OO stuff!
Alexander Beletsky
I'm also going to learn F#.. But I would like to start with fundamentals, istead of language itself.

I'm considering this book to start:

http://mitpress.mit.edu/sicp/full-text/book/book.html

Its availble in pdf, as well :)

Whats Coming in MvcMailer NuGet 1.0?

Thanks to 365+ downloads of the MvcMailer NuGet before it hit version 1.0 and now its time to wrap up the NuGet package for an official first release. I got several encouraging feedback on this package and here’s what you get from this first release:
  1. Use Scaffold Mailer to generate your mailers with views.
  2. Compose rich emails using your favorite ASP.NET MVC view engine.
  3. Use master pages and pass data using ViewData/ViewBag.
  4. Send multi-part emails for both text and html email readers.
  5. Put images inline so that they are visible even when email client is offline.
  6. Write unit test for your mailers and controllers that use the mailers.
  7. Send attachments.
  8. Send emails asynchronously.
To my knowledge, MvcMailer is gonna be the first full stack ActionMailer like Emailing library for ASP.NET MVC 3. If you are an ASP.Net MVC programmer who deeply care about high quality work and maximizing efficiency - you gotta try MvcMailer 1.0 for writing pretty email sending code.
The github wiki page has everything you will need to know about MvcMailer. Version 1.0 comes out on Monday. Stay tuned.

ActionMailer 3 - Why Do You Call Instance Methods as Class/self Methods?

I didn’t even notice this little trick! As long as I didn’t have to call deliver_welcome_message (or deliver_*) methods that would magically call welcome_message, I was happy that now the magic is gone. Things are transparent!
Here’s an example showing the change: Say you have the following mailer:
class Notifier < ActionMailer::Base
def welcome_message(new_user)
#a warm welcome message
end
end
Now, prior to Rails 3, or ActionMailer 3, you would write the following to actually call this method to get the benefits of ActionMailer magics, such as finding the view based on method name and so on:
Notifier.deliver_welcome_message(new_user_instance)  
I am sure this deliver_* was a clever design decision to solve a hard problem, that is, finding the view name based on the method name. However, in ActionMailer 3, this is gone. Now the question is, if this trick is gone, how come it still finds the view name from the method name? Who sets the view name? To know the answer, first, let’s take a look at how we call the welcome_message now.
Notifier.welcome_message(new_user_instance).send
Instead of

Notifier.new.welcome_message(new_user_instance).send

So, the magic deliver_ prefix is gone. But, did you see the new trick? Well, its a clever design again. The trick this time is, you call your instance method, welcome_message as if it was a class method. But there is no class method called welcome_message, so it instead goes to method_missing and thats how it sets up the view name from this call. Here’s the code that does this little trick!
def method_missing(method, *args) #:nodoc:
return super unless respond_to?(method)
new(method, *args).message
end
All it does is, instantiates the mailer with the method name!

However, this design decision has interesting side effects as well. Or may be not side effects, but rather core effects. For example, since you are calling your mailer methods as class methods, you cannot use a single mailer instance to send out multiple emails at the same time. In fact, every mailer has only one instance of message. So, it cannot store two messages at the same time. This is as if, you can have multiple methods in a class, but you cannot call more than one class or you will mess up the class’s state!

Wonder why? Well, this is rooted in another key design choice: ActionMailer::Base is a subclass of AbstractController::Base. Now, if you look at controllers, you will notice that at any given point of time, a controller instance is only responsible for responding to a single action. This is logical for controllers. But how about mailers? I see a mismatch in my mental model and the actual implementation model. I don’t see a reason why a mailer is a controller! For the sake of code reuse? But that could be done via delegation anyway.

I will end this post with one question:
Do you think mailer is a controller? hints: think about LSP.

My Article at CodeProject: MvcMailer

I just released a .Net NuGet package called MvcMailer and to get people super easily started, put an article at CodeProject.com. You are most welcome to the article at http://www.codeproject.com/KB/aspnet/MvcMailerNuGet.aspx
I welcome your comments and suggestions!

Comments

Sohan
Just released 0.8 and it should work. But please let me know if it doesn't.
I also added multi-part emails in version 0.8. You may check that out as well.
One important change:
mail.Body = PopulateBody(mail, "welcome") needs to be replaced with PopulateBody(mail, "Welcome"), it gets rid of the duplication
Sohan
This comment has been removed by the author.
Jakub
Brilliant! Can't wait. Could you post a comment here or send me a line to jkonecki [at] gmail.com when the new release is out so I can test it as well?
Sohan
Thanks for your reply. I will test this out before I push a new release. The new release will also add multi-part emails so that you can easily send html and plain text emails.
Jakub
My route is defined as:

routes.Add("Account", new Route("{account}/{controller}/{action}/{id}",
new RouteValueDictionary(new { account = "", controller = "Home", action = "Index", id = "" }), new AccountMvcRouteHandler()));

I'm trying to render a link:
href="@Url.Abs(Url.Action("About", "Home"))"

The url that is generated looks like:

http://mysite/Home/About

instead of

http://mysite/myaccount/Home/About
Sohan
Can you please share your specific route and ActionLink in mailer view that breaks for the route data?

I know a solution to your problem now. But I would like to make sure I know the problem in detail.
Jakub
Great! I'm waiting impatiently… ;-)

PS. Congratulations on joining ThoughtWorks
Sohan
Yes, a new version is overdue now:)
Jakub
Awesome!

Will you release a new version through NuGet? I will be able to upgrade easily and test it for you.

Cheers!
Jakub
Sohan
Thanks for your suggestion. I will fix this issue this weekend.
Jakub
Hi!

Thirst of all, thank you for a brilliant package! It's simple and easy to use!

I've just found one little issue - I've notices that the ControllerContext for Notifier doesn't have the RouteData dictionary populated with the same values as the RouteData for main controller. This causes UrlHelper to generate invalid Urls. My app is a multi-tenant one and all my routes have format: /{account}/{controller}/{action}/ I've managed to 'fix' it by adding following code in the WelcomeMessage.cshtml

@{
this.Url.RequestContext.RouteData = (this.Context.CurrentHandler as MvcHandler).RequestContext.RouteData;
}

I wonder if you could fix it properly, then MvcMailer will be just perfect! ;-)

Thank you,
Jakub

C# Named Parameters Can Be Very Useful

Think about the following code example:
htmlHelper.InputHelper(InputType.CheckBox, name, “true”, true /* useViewData */, false /* isChecked */, true /* setId */, false /* isExplicitValue */, htmlAttributes);
This line is taken from the source code of ASP.NET MVC, or to be more specific its a line from the InputExtension class. I think the intent of the original developer is clear:
Add comment next to arguments so that one can easily read the method call without getting lost in a who’s who since there are quite some arguments to pass.
To learn more about named parameters using C#, you can visit this post by ScottGu.