Pure Lunacy or Sheer Genius, You Decide.

I've been doing a code review for a new Microsoft Dynamics CRM 4.0 based application that is coming in and today I found the method below:

   1 
   2     [WebMethod]
   3     public string Engaged(string oId, string strValues)
   4     {
   5         try
   6         {
   7             LogEntry("TRACE", "WebService : Engaged", String.Format("Start: Call oId: {0}, strValues: {1}", oId, strValues));
   8             call outcall = new call();
   9 
  10             CrmBoolean boolean = new CrmBoolean();
  11            
  12             String engagedValue = strValues.Substring((strValues.Length - 3), 2); //Request.QueryString["oId"].Substring((Request.QueryString["oId"].Length - 2), 2).ToString();
  13             if (engagedValue == "No")
  14             {
  15                 boolean.Value = true;
  16             }
  17             else
  18             {
  19                 boolean.Value = false;
  20             }
  21             LogEntry("TRACE", "WebService : Engaged", string.Format("1. Set Call Engaged Status to {0}.",boolean.Value));
  22             outcall.engaged = boolean;
  23 
  24             Key key = new Key();
  25             key.Value = new Guid(oId); 
  26             outcall.callid = key;
  27 
  28             LogEntry("TRACE", "WebService : Engaged", "2. Update Call record");
  29             crmservice.Update(outcall);
  30 
  31              LogEntry("TRACE", "WebService : Engaged", "Return: All Done.");
  32 
  33             return "";
  34         }
  35         
  36         catch (Exception e)
  37         {
  38             LogEntry("ERROR", "WebService : Engaged", string.Format("Error in Engaged Engaged.  Exception {0}.", e.ToString()));
  39 
  40             return e.ToString();
  41         }
  42     }

This method gets invoked with a isv button on the home page. Call here is actually a custom entity.  oId contains the Call guid and strValues, I was not prepared for this, contains a string that is generated like this:

   1 if (document.all.crmGrid.InnerGrid.SelectedRecords.length > 0)
   2 {
   3 var url = "/WebService.asmx/Engaged";
   4 var oXmlHTTP = new ActiveXObject("Msxml2.XMLHTTP");
   5 oXmlHTTP.Open("POST", url, false);
   6 oXmlHTTP.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
   7 oXmlHTTP.Send("oId=" + document.all['crmGrid'].InnerGrid.SelectedRecords[0][0] +"&strValues=" + document.all['crmGrid'].InnerGrid.SelectedRecords[0][3].innerText);
   8 document.getElementById('crmGrid').Refresh();
   9 }

Oh, yes we are screen scraping, in our own App, for which we have full control over everything, a perfectly good SDK and is actually all hosted within the same datacentre and inside a secure network, accessed by a single company.

You might be inclined to think that this is a response to a very slow network and that by skipping a call to the database to get the Call instance that we are updating, we might make the user experience better. I would be willing to concede on this point, if it wasn't for the rest of the code that makes calls to the database with careless abandon.

I won't even go into returning an empty string, how the entity has different views (some of which don't contain the engaged attribute) or the fact that if it fails the user is not notified in any way.

So what do you think, Pure Lunacy or Sheer Genius?