Sponsored By Aspose - File Format APIs for .NET

Aspose are the market leader of .NET APIs for file business formats – natively work with DOCX, XLSX, PPT, PDF, MSG, MPP, images formats and many more!

Microsoft’s own Silverlight demo showcases how NOT to code

After stumbling with the latest install of Orcas, I loaded up the Silverlight 1.1 chess demo and was blown away with the poor quality of the coding style used in the demo. I’m at a total loss as to how a public demo comes out like this. It truly saddens me because for every 1 shitty demo Microsoft puts out, it probably takes 100 good ones to even out.

Am I being too harsh? Judge for yourself:

1 –
Multiple public types per file  which is a direct violation of Microsoft’s INTERNAL Design Guidelines.

2 –
The namespace is “Chess”, which also violates Microsoft’s Design Guidelines.

3 –
The boardui.cs, browser.xaml and default.html.js files are all lowercase – all other files are PascalCased

4 –
Some simple getters are written in a single line, others over multiple lines

5 –
Some types and members rely on implicit visibility (don’t specify private/internal/public), others specify it. I don’t even know what the default type is…isn’t it internal for classes and private for fields?

6 –
Fields are spread all over…most are at the top of the class definition, some are defined above a method half way down.

7 –
Single line case statement which violates Microsoft’s Internal design guidelines

8 –
Classes with only static members that aren’t sealed and don’t have a private constructor (or, in 2.0, aren’t marked as static)

9 –
An unbelievable amount of SET-only properties (again, like most of these, violates Microsoft’s own design guidelines)

10 –
Within the same method, some fields are accessed with “this.” others aren’t

11 –
Some 1 line IF’s/Loops have braces…some don’t

12 –
Properties that call fairly expensive methods

13 –
Some usings are declared inside the namespace, some declared outside

14 –
Many if statements would benefit from Guard Clauses and Decompose Conditional refactoring

15 –
Poor use of #regions and very little of them
 

There’s certainly more. Many of the items above (especially #1 and #6) make the sample very hard to read. Running the project through FxCop came up with 143 issues. 

I want to cry. 

 

This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.

24 Responses to Microsoft’s own Silverlight demo showcases how NOT to code

  1. karl says:

    André you’re absolutely right. This goes into being a professional at what you do, rather than a Mort.

    Yes there’s a reality…there are tight deadlines, but all that does is [partially] excuse the individual directly responsible for the code…it doesn’t excuse the PM or the VP who’s impossible-to-meet deadline compromised the system.

  2. André says:

    But if you are cursorily and messy while writing code and lots of you are argumenting to spend some time later to clean it up, then the question is just: why are you not writing it “clean” from the beginning, it would take you just a trickle longer than doing it afterwards. This is a principle of organization (not only regarding coders)!

  3. Todd P says:

    Hmm….I agree with the above, when you’re rushing something out the door for demo purposes and the technology behind the scenes is changing fast, there’s a lot more important things to be thinking about than cleaning up your code, like how does that new Brush class that got thrown into last night’s build work?
    For file’s being named in a mixture of all lower-case and some upper case. What if the very beta version of the tools Created the boiler plate files in a Camel Case way, but then the developer was given a helper file from another developer that was lower-case? I see so many reason’s for some sloppy things on tight deadlines with changing infastructure. I just don’t see the importance or need to care about how files are named for something like this.
    Now, if it’s an application being pushed into production with hundreds of classes and multiple-people editing the files…that’s a different story.
    And namespace “Chess” violating guidlines.
    Also, this is a Chess application, it’s not like there’s some good boiler-plate code in there that you’re going to copy out and use in your application.

  4. Todd P says:

    Hmm….I agree with the above, when you’re rushing something out the door for demo purposes and the technology behind the scenes is changing fast, there’s a lot more important things to be thinking about than cleaning up your code, like how does that new Brush class that got thrown into last night’s build work?
    For file’s being named in a mixture of all lower-case and some upper case. What if the very beta version of the tools Created the boiler plate files in a Camel Case way, but then the developer was given a helper file from another developer that was lower-case? I see so many reason’s for some sloppy things on tight deadlines with changing infastructure. I just don’t see the importance or need to care about how files are named for something like this.
    Now, if it’s an application being pushed into production with hundreds of classes and multiple-people editing the files…that’s a different story.
    And namespace “Chess” violating guidlines.
    Also, this is a Chess application, it’s not like there’s some good boiler-plate code in there that you’re going to copy out and use in your application.

  5. Nate Kohari says:

    I guarantee the demos were rushed out the door in order to demonstrate Silverlight. Microsoft never said that the code reflects best practices. It’s asking a little too much, in my opinion, to have proof-of-concept demo written in two weeks to demonstrate alpha software be well-written — and I’m a standards nazi by nature. :)

  6. nuLl says:

    man, this evening go to your local bar and have a couple of drinks, really

  7. Aly says:

    I think you guys here are missing the point. There should be someone doing Quality Control and what MS puts out should be reviewed by someone senior is the assumption that poor coders and interns are used for these purposes is true. Other than than, it makes pretty good sense why its hard for MS to resolve issues in their products and why its also very important that they maintain the closed source stance.

    I bet most of the brilliant coders have issues with doing all these aesthetics to code

  8. karl says:

    Garren and to the others defending Microsoft….don’t you just naturally code properly? You can rush me all you want, but I’ll still only put 1 public type per file, and make sure my fields aren’t spread out everywhere.

    I think Brad Abrams has it wrong. Yes it’s great that they’ll revisit it, but there’s a root problem that ought to be fixed.

    I’m sure the code will get fixed in a couple months, but why is making code readable something that doesn’t happen as you’re writing it? Even the crap I write in Code Snippet follows some order.

  9. Garren says:

    I was at MIX07 and as I recall more than a few of the demos were put together within a two-week timeframe. The Ruby-Silverlight REPL running on a Mac is a good example. Two weeks. Please take these examples for what they actually are: Demo code to intended to demonstrate BETA TOOLS.

    Incidentally, orcas crashed in virtually every session that I was in — even the keynote iirc. My point is that these aren’t finished products by any stretch so don’t expect the demo-code to be a shining beacon of the Microsoft Method.

  10. karl says:

    for you region haters, the main complaint is how inconsistent they are. I’m hopeful that you would agree consistent regions are better than inconsistent ones.

    JC:
    If out of 14 points I make 1 you don’t agree with and I’ve lost all credibility, then you’re high horse is higher than mine :)

    John:
    It’s a demo to learn how to code. If it was a demo of just what silverlight can do, they wouldn’t give the source. Releasing poor source code reflects bad on the product and the company, but more importantly wastes my time.

  11. jc says:

    Your post lost credibility to me when you mentioned
    “Poor use of #regions and very little of them”

    Give me a break. Regions are a disaster to coding. They shouldnt be anywhere. If you find yourself needing to use them, its a sign that your class has become an unmanagable mess.

  12. Regions are a mess to begin with, and it isn’t surprising that people have subjective opinions about how they *should* be done.

    Most everything else you mentioned are superficial and completely ignorable. With all due respect, when people throw up their arms over such trivia of code, I immediately suspect their skill, and it casts them more as a “coding assistant” than an actual professional.

  13. Lee says:

    I prefer to obvuscate my code….Much better job security when no one else understands your code. I guess the SilverLight developers want the same :-)

    Regards
    Lee

  14. Carl says:

    I agree with you Karl and also the commenter Stu – Demo code should be the most readable code of all. If I was putting out code for public viewing I would spend the hour or two required to pretty it up including conforming to the styleguide, adding extra comments, adjusting whitespace to visually separate components (if putting multiple elements in one file), etc. That stuff doesn’t take long, but it makes a big difference to a stranger looking at it.

  15. Come down off the ledge, man. It’ll all be ok. It’s a demo of silverlight, not a demo of the design guidelines which are primarily intended for class libraries and public API’s anyway.

    I write pretty good code when I want to (for the larger applications). But I don’t take the time to make sure everything is neat and consistent when I’m writing quick utilities and sample code. That’s life. Time is money.

  16. Paul says:

    Your right but all that can be cleaned up. The most important thing to me is that bad coders were able to do RAD and get it working as proof of concept and it worked. Others will surely disagree.

  17. Brad Abrams says:

    Thanks for your comments! You are right that the code for this demo needs some work… we will invest in getting it cleaned up for a future release…

    Feel free to drop me an email about any other issues you see and keep the comments comming…

  18. Joe Ocampo says:

    See what happens when “designers” code! :-)

  19. Mr_Demo says:

    For me, I use demo code as a learning tool and if you think the demo is crap, well…

  20. Jason Bock says:

    I would agree with your gripes, other than the regions thing. I HATE regions – they’re usually abused to hide massive amounts of code.

  21. Stu Smith says:

    Karl I completely agree, it’s one of my personal bug-bears. I used to get really annoyed with my developers if they checked in what I thought was “messy” code. I still haven’t got a decent argument about why I think it’s important to have “clean” code though, other than vague ones about it being easier for other developers to read. I think quite a few people thought I was just being obsessive-compulsive about it all (and I used to worry that perhaps I was).

    Demo code I think has the ultimate requirement to be both easy and pleasant to read – afterall it serves no other purpose other than to be read. Sloppy layout to me implies sloppy code overall.

    I’m sure that one day source code in text form will be a thing of the past – I think eventually it will be stored as parse trees and we can all format it how we like – an end to the religious syntax wars.

    (Aside #1 – I’d like to see more demo code written in the style of Fraser and Hanson – literate code written as code and prose intertwined.)

    (Aside #2 – I’d also like to see more stuff like Hackety Hack – MS could learn a thing or two there).

  22. karl says:

    Mark, you don’t find that if the style makes the demo hard to read (some of the classes have 5+ public types and 3+ different places where private fields are declared), that it somewhat defeats the purpose of the demo?

  23. Matt says:

    That’s been the case for Microsoft example code for as long as I remember (Windows 2.0). I guess that if you’re skilled enough to write good code, Microsoft would rather you developed products, so demos get done by interns and poor developers.

  24. Mark Cohen says:

    I think just maybe you take demo downloads a pinch too seriously.