6
\$\begingroup\$

As a test of my C# skills, I have been asked to create a simple web service which will take a message from a queue table in a SQL database and send it to a web application when a button is pressed on the application. I have never written a web service before so I am going in a little blind here so was after some advise on whether I had done this correct or not.

The stored procedure usp_dequeueTestProject gets a value from the top of the list in a table and then deletes the row in that table. At the moment I do not have this being archived anywhere, would it be better practice to instead of delete this, to just mark it as sent instead?

[WebService(Namespace = "http://tempuri.org/")]
[WebServiceBinding(ConformsTo = WsiProfiles.BasicProfile1_1)]
[System.ComponentModel.ToolboxItem(false)]
// To allow this Web Service to be called from script, using ASP.NET AJAX, uncomment the following line. 
// [System.Web.Script.Services.ScriptService]
public class Service1 : System.Web.Services.WebService
{
    [WebMethod]
    public string GetDataLINQ()
    {
        try
        {
            TestProjectLinqSQLDataContext dc = new TestProjectLinqSQLDataContext();
            var command = dc.usp_dequeueTestProject();
            string value = command.Select(c => c.Command).SingleOrDefault();
            return value;
        }
        catch (Exception ex)
        {
            return ex.ToString();
        }
    }
}

I've opted for using LINQ for starters, I am not sure if this is the best way to do it or not? I am only passing through the one string as well... In reality I guess you would normally want to send more than one field, such as datetime sent, message type etc, but wasn't sure what data type to use for this? I have seen this done using a Strut, but wasn't sure if this was correct? Any guidance would be greatly appreciated.

\$\endgroup\$
  • 1
    \$\begingroup\$ What is the nature of this "skills test"? Seems like asking the internet for a better solution won't be a good indicator of your skills. \$\endgroup\$ – Ocelot20 May 21 '14 at 18:48
  • \$\begingroup\$ Just to see if I'm able to do a web service really.. I want to impress though so don't want to muck it up by doing something glaringly obvious. \$\endgroup\$ – user3284707 May 21 '14 at 18:52
  • \$\begingroup\$ I mean who is administering the test? A potential employer, a teacher, yourself? If it's an employer who wants to know if you know how to properly write a web service, then getting help from the internet isn't really an indicator of your skillset. \$\endgroup\$ – Ocelot20 May 21 '14 at 18:56
  • \$\begingroup\$ It's a work project where a number of us have been asked to produce this simple web service. My boss knows I have very limited experience with web services so she will be expecting me to get help. I am more after some feedback on the code I have written and if it's the correct way to go about it, rather than someone giving me a solution which is better. Some quick suggestions on the questions I have asked would be greatly appreciated. \$\endgroup\$ – user3284707 May 21 '14 at 19:10
4
\$\begingroup\$
  • A Linq-to-SQL DataContext is an IDisposable, your code isn't calling its Dispose() method. Wrap it in a using block to ensure proper disposal.

  • I would use var in the TestProjectLinqSQLDataContext instantiation as well, but that's just personal preference. I find Foo bar = new Foo() redundant.

  • Using SingleOrDefault assumes that the SP is returning only 1 record. That very well be the case now, but the SP being outside of the code, I wouldn't make that assumption. Use FirstOrDefault to grab only the first record - this way the day the SP is modified to return more than 1 row, your code won't break.

Something like this:

string result;

using (var dc = new TestProjectLinqSQLDataContext())
{
    var command = dc.usp_dequeueTestProject();
    result = command.Select(c => c.Command).FirstOrDefault();
}

return result;

I agree with @Jesse's comment about exceptions usage: the calling code has no way of easily telling a valid response from an error message (exception type, message and stack trace), since your code returns a string in every case.

Let it blow up!

\$\endgroup\$
  • \$\begingroup\$ Thank you for your comment. Should I wrap all my linq code in using so that it properly disposes of the code after use? Thank you for your comment about the FirstOrDefault as well I will change this in my code. Could you advise whether a strut would be best to do a web service which returns multiple fields? I am going to look at pulling through more fields as I imagine this is what we will be doing in a future project. \$\endgroup\$ – user3284707 May 21 '14 at 20:30
  • \$\begingroup\$ You mean a struct? I'd use a class instead, a POCO (Plain Old CLR Object) - a simple class that merely exposes auto-properties should do the trick. \$\endgroup\$ – Mathieu Guindon May 21 '14 at 20:33
5
\$\begingroup\$

LINQ is a good choice. You're querying something, so it lends itself to the task.

That being said, the exception handling is atrocious. Making your return values null, some command's value from the database or some big honkin' exception text is abusing the string data type. I'd remove the try..catch altogether and let your client app handle the exception if it knows what to do. In the best case, your catch could log the error somewhere then re-throw.

\$\endgroup\$
  • \$\begingroup\$ Thank you very much for your reply. I will take out the try catch as you say. Can you advise what you think I should use if I transfer more than just a string over the webservice? I've done it with a strut, but I wasn't sure if that was the most elegant way to do it? \$\endgroup\$ – user3284707 May 21 '14 at 19:58
  • \$\begingroup\$ What do you want to transfer? Use any data type you like and it will be serialized as you expect. \$\endgroup\$ – Jesse C. Slicer May 21 '14 at 20:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.