Windows Phone 8 web service app classes

Question

I have a classes for working with a web service (Windows Phone 8 app). My main question is about the thread-safety of the solutions. I think there will be problems with a static class GYApi. After all, static classes aren't inherently thread safe. But not only in a static class.

If synchronization is necessary for locking, then how it will affect the performance?

GYResponseGeneric is an open type for deserializing JSON to objects:

    [DataContract]
    public class GYResponseGeneric<T>
    {
        [DataMember(Name = "code")]
        public int Code { get; set; }

        [DataMember(Name = "message")]
        public string Message { get; set; }

        [DataMember(Name = "result")]
        public T Result { get; set; }
    }

Helpers class for passing some parameters:

public class RequestGeneric<T>
    {
        public HttpWebRequest HttpRequest { get; set; }
        public string Url { get; set; }
        public string PostData { get; set; }
        public Action<GYResponseGeneric<T>, Exception> CallBack { get; set; }
    }

PostRequester is a singleton. This class sends post data to a web server (HttpWebRequest), gets JSON from the web service and deserializes it. It invokes callback delegate .Invoke(response, error).

public sealed class PostRequester
    {

        static PostRequester instance = null;
        static readonly object padlock = new object();


        public static PostRequester Instance
        {
            get
            {
                lock (padlock)
                {
                    if (instance == null)
                    {
                        instance = new PostRequester();
                    }
                    return instance;
                }
            }
        }

        private PostRequester() { }     

        public void SendPost<T>(string postData, Action<GYResponseGeneric<T>, Exception> callback)
        {
            Task.Factory.StartNew(() => SendPostThread<T>(new RequestGeneric<T>
            {
                CallBack = callback,
                PostData = postData,
            }));
        }    

        public void SendPostThread<T>(RequestGeneric<T> request)
        {
            ...
            webRequest.BeginGetRequestStream(new AsyncCallback(GetRequestStreamCallback<T>), new RequestGeneric<T>()
                    {
                        HttpRequest = webRequest,
                        PostData = request.PostData,
                        Url = UrlRequest,
                        CallBack = request.CallBack,
                    }); 
             ...              
        }

        private void GetRequestStreamCallback<T>(IAsyncResult asynchronousResult)
        {
             ...
             // Start the web request
            request.HttpRequest.BeginGetResponse(new AsyncCallback(GetResponseCallback<T>), request);
             ...
        }

        private void GetResponseCallback<T>(IAsyncResult asynchronousResult)
        {
             using (MemoryStream stream = new MemoryStream(Encoding.Unicode.GetBytes(gyResponse)))
            {
                DataContractJsonSerializer serializer = new DataContractJsonSerializer(typeof(GYResponseGeneric<T>));
                GYResponseGeneric<T> responseObj = (GYResponseGeneric<T>)serializer.ReadObject(stream);

                if (request.CallBack != null)
                {
                     //get response Code (status response), Invoke callback etc
                }
            }
        }

Static class for creating Post data and passing it to PostRequester:

public static partial class GYApi
{
    ...
    public static void GetInfo(string uid, Action<GYResponseGeneric<GYUser>, Exception> callback)
    {
        string model = "User";
        string method = "getInfo";
        Params parametrs = new Params();
        parametrs.Uid = uid;           
        string request = PostRequester.Instance.GetRequestString(model, method, parametrs);
        PostRequester.Instance.SendPost<GYUser>(request, callback);
    }

    public static void SetLocation(GeoCoordinate location, Action<GYResponseGeneric<bool>, Exception> callback)
    {           
        string model = "User";
        string method = "setLocation";
        Params parametrs = new Params();          
        parametrs.Latitude = (float)location.Latitude;
        parametrs.Longitude = (float)location.Longitude;
        string request = PostRequester.Instance.GetRequestString(model, method, parametrs);
        PostRequester.Instance.SendPost<bool>(request, callback);
    }
    ...
 }

Example of calling API:

GYApi.GetInfo((response, error) => 
{
   if (error != null)
   {
      ...
      return;
   }

   GYUser user = response.Result;

})

Answer 1

1. I don't see any reason why PostRequester should be a singleton, just make it a static class.
2. Since PostRequester doesn't contain any fields, you don't have to worry about its thread-safety. Unless you forgot to include those, because I don't see where UrlRequest is coming from.
3. When you're using asynchronous methods (like BeginGetRequestStream()), there is usually no need to also run it on a separate thread using Task.Factory.StartNew().
4. If you can use C# 5.0, consider switching to async-await for your asynchronous methods.
5. I don't like the way you're using GetRequestString() much. I think a better way would be to have one method that builds the request string and then also posts it.
6. When you create a new object and then assign its properties, you can use object initializer. For example: new Params { Uid = uid }.

If for synchronization necessary use lock, then how it will affect the performance?

The general rule of performance optimizations applies here:

1. At first, don't worry about performance, just write correct readable code.
2. If it turns out your code is slow, find out which part is responsible, and optimize that.

Specifically with lock, it does have some overhead, but it shouldn't affect performance much, under normal circumstances.

Answer 2

In addition to what @svick wrote:

1. It is obvious from the generic type parameter that a class is generic - no need to spell it out in the name of the class (i.e. GYResponseGeneric should be GYResponse and RequestGeneric should be Request).
2. You expose both SendPost and SendPostThread as public. From the code SendPost should be named SendPostAsync because that's what it does. SendPostThread seems to perform the synchronous request and it should therefore be be named SendPost.
3. If you want to expose both SendPost methods public then you should consider making their interfaces more consistent - one expects a string for the data, the other one a request object. Otherwise declare the synchronous method private.
4. You only need to synchronize access to resources which can be accessed simultaneously by multiple threads. As far as I can see your static methods (when you make PostRequester static) are all self contained and do not share any data so you don't need to put any locks in (in the code you have shown).

5. If you insist on keeping PostRequester a singleton then you can just use the class initializer for Instance then you don't need any lock:

   public static readonly PostRequester Instance = new PostRequester();