Backspace Away!

20. October 2010 03:38 by rencarnacao in General  //  Tags:   //   Comments (0)
Backspace Away!

It’s no wonder why the urge to comment out code that isn’t migrating well and anything outside of the initial automation results - is so strong. It’s like trying to close a suitcase that’s clearly over-stuffed. You want to close it up with everything you originally envisioned packing into it but some stuff just doesn’t seem to fit. In cases like these, it can be difficult to decide what to keep and what to toss out. And like most, traveling light and simple, is preferred. If life’s problems where only a backspace away from being resolved. Don't like your electric bill? Delete it. Neighbor's dog barks too loud at night... well, you get the point.

And so the options that I’ve found myself debating lately have been whether to comment out these lines of code and possibly throw a custom exception or to create a precompiler warning. While both postpone the time at which the possible underlying issues are addressed, they present the developer with completely different opportunities. And only one of them gets you closer to lift off quicker.

Throwing an error at runtime is highly effective at ensuring that possible migration issues are not ignored or missed entirely - especially after your QA team begins testing. The test cases might not produce the necessary conditions for those lines to run and thus give Visual Studio a reason to throw your exception. While they, run-time exceptions, are no longer compile-time errors, they could make their way to the final product - surprising users with potentially bizarre comments and buggy functionality, - like the one below.

   1:  if (confirmPaymentDialogResult == DialogResult.Ok)
   2:      {  
   3:           //CheckoutCart Object is still using the Interop version. 
   4:           //PaymentsFullfillmentObject.ChargeAmout(AxCheckoutCartObject.TotalAmount());
   5:           PaymentsFullfillmentObject.ChargeAndShip();
   6:           throw new Exception("Don't forget to fix this issue!");
   7:       }

However, I think its important that when making modifications to the code like this, it’s critical to look at the context of the user experience and what exactly you’re commenting out. If you’re going to comment out the use of the AxCheckoutCartObject above than it’s probably best to also comment out the ChargeAndShip() method.

One of the problems with this though is that it’s possible to forget about this issue until someone from your QA team throws a hissy fit and demands to know how the module was ever tested. But wait... aren't developers are notoriously awesome testers!?

I’ve noticed since I’ve been working with Artinsoft, that one of the many neat benefits of using the VB Companion is some of the migration code that’s placed in the source code to assist with the migration.

   1:  //UPGRADE_TODO: (1065) Error handling statement (On Error Resume Next) could not be converted.     
   2:  //More Information:
   3:  Artinsoft.VB6.Utils.NotUpgradedHelper.NotifyNotUpgradedElement("On Error Resume Next");

The example above is taken from an instance where VB Companion actually created and inserted this NotifyNotUpgradedElement() method. What this means is that while you are debugging, you’ll be notified about the conversion issue whenever the application hits that line of code. This offers considerable benefits especially since it allows you to ignore all of those NotifyNotUpgradesElement calls and get back to them later.

So I feel like I’ve talk myself out of using the throw exception and to replace it with the NotifyNotUpgraded() method approach. But I think we can still do more to ensure that issues aren’t missed entirely or caught post-development by your QA team.

I’d like to be notified of these issues regardless of whether the lines are ever executed. So what I’ve discovered recently is the use of the warning directive.

   1:  #warning Artinsoft: Connection mode should be set to 'ModeRead'.

This will show up between the error list and the information list tabs. We just need to remember to check these warnings.


So now, if we incorporate the warning directive and the NotifyNotUpgradedElement() call then the “throw Exception” really isn’t needed any longer and we’ll increase the likelihood of remembering to go back addresses issues that were tabled for whatever reason.

So the previous code example might end up looking like this:

   1:  if (confirmPaymentDialogResult == DialogResult.Ok)
   2:      {  
   3:           //CheckoutCart Object is still using the Interop version. 
   4:           //PaymentsFullfillmentObject.ChargeAmout(AxCheckoutCartObject.TotalAmount());
   5:           Artinsoft.VB6.Utils.NotUpgradedHelper
   6:  #warning Artinsoft: PaymentsFullfillmentObject.ChargeAndShip()
            needs to be replaced with .Net equivalent.
   7:           PaymentsFullfillmentObject.ChargeAndShip();
   8:       }

This approach provides three benefits. It allows the developer to comment out lines of code that are throwing exceptions during compilation or debugging without worry about forgetting about them. This in turn will get the migrated application up and running quicker. Additionally, during each debugging session, it optionally allows the developer to see each issue as the application encounters them or skip over all of them completely. And finally, while compiling, it provides a list of these issues as warnings in the error list tab.

This approach is proving to be quite useful.