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.
-
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– Ocelot202014年05月21日 18:48:12 +00:00Commented May 21, 2014 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– user32847072014年05月21日 18:52:36 +00:00Commented May 21, 2014 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– Ocelot202014年05月21日 18:56:15 +00:00Commented May 21, 2014 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– user32847072014年05月21日 19:10:29 +00:00Commented May 21, 2014 at 19:10
2 Answers 2
A Linq-to-SQL
DataContext
is anIDisposable
, your code isn't calling itsDispose()
method. Wrap it in ausing
block to ensure proper disposal.I would use
var
in theTestProjectLinqSQLDataContext
instantiation as well, but that's just personal preference. I findFoo 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. UseFirstOrDefault
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!
-
\$\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– user32847072014年05月21日 20:30:02 +00:00Commented May 21, 2014 at 20:30
-
\$\begingroup\$ You mean a
struct
? I'd use aclass
instead, a POCO (Plain Old CLR Object) - a simple class that merely exposes auto-properties should do the trick. \$\endgroup\$Mathieu Guindon– Mathieu Guindon2014年05月21日 20:33:38 +00:00Commented May 21, 2014 at 20:33
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.
-
\$\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– user32847072014年05月21日 19:58:42 +00:00Commented May 21, 2014 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– Jesse C. Slicer2014年05月21日 20:29:27 +00:00Commented May 21, 2014 at 20:29
Explore related questions
See similar questions with these tags.