Guilty of unsound programming

Question

I was reading Robert Rossney's entry on "What's the most unsound program you've had to maintain?" found at: (What's the most unsound program you've had to maintain?) when I realized that I had inadvertently developed a near-identical application!
The app consists of an HTTPListener object that grabs incoming POST requests. Based on the information in the header, I pass the body of the request to SQL Server to perform the appropriate transaction.
The requests look like:

<InvoiceCreate Control="389>
  <Invoice>
    <CustomerNumber>5555</CustomerNumber>
    <Total>300.00</Total>
    <RushOrder>1</RushOrder>
  </Invoice>
</InvoiceCreate>

Once it's received by the HTTPListener object, I perform the required INSERT to the Invoice table using SQL Server's built-in XML handling functionality via a stored procedure:

  INSERT INTO Invoice (InvoiceNumber, CustomerNumber, Total, RushOrder)
  SELECT @NEW_INVOICE_NUMBER,  
         @XML.value('(InvoiceCreate/Invoice/CustomerNumber)[1]', 'varchar(10)'),
         @XML.value('(InvoiceCreate/Invoice/Total)[1]', 'varchar(10)'),
         @XML.value('(InvoiceCreate/Invoice/Total)[1]', 'varchar(10)')  

I then use another SELECT statement in the same stored procedure to return the value of the new Invoice Number that was inserted into the Invoices table:

SELECT @NEW_INVOICE_NUMBER FOR XML PATH 'InvoiceCreateAck'  

I then read the generated XML using a SQL data reader object in C# and use it as the response of the HTTPListener object.

My issue is, I'm noticing that Robert is indeed correct. All of my application logic exists inside the stored procedure, so I find myself having to do a lot of error-checking (i.e. validating the customer number and invoicenumber values) inside the stored procedure.

I'm still a midlevel developer, and as such, am looking to improve. Given the original post, and my current architecture, what could I have done differently to improve the application? Are there any patterns or best practices that I could refer to? What approach would you have taken? I'm open to any and all criticism, as I'd like to do my part to reduce the amount of "unsound programming" in the world.

Answer 1

Not sure about specific patterns, but you need to define your layers and stick to it. I'm using layer very loosely here. You have one layer which needs to parse XML. You have one layer which reads a HTTP request. You have one layer which does data access. If you're using C#, these are likely 3 separate classes.

I would not do it in the stored procedure. From my experience, doing this stuff makes it almost be a complete rewrite if you change databases. Testing is really difficult too.

For your data access, you could still use your stored procedure. I personally would just write simple insert statements using C#. I would just strive to keep any business logic out of the stored procedure. Keep any business logic, parsing in C# as this will be much easier to port later.

Good luck!