Web Service Handler

Question

My webservice class:

public class ConnectionExector {

    private Context context;
    private static final int WAIT_RESPONSE_TIMEOUT = 3000;
    private static final int CONNECTION_TIMEOUT = 30000;
    protected ExecutorService executorService;
    // to handle thread result
    Handler handler = new Handler();
    // resposes type 
    public enum ResponseType {
        SUCESS, ERROR, TIMEOUT, NULL;
    }

    public class HTTPResponse {
        public ResponseType response = ResponseType.NULL;
        public String value = "";
        public boolean cashed = false;
        public Object data = null;
    }

    public interface onHttpRequestListener {
        public void onHttpResult(HTTPResponse response, String senddata);
    }

    public ConnectionExector(Context context) {

        try {
            this.context = context;

            executorService = Executors.newFixedThreadPool(3);

        } catch (Exception e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }

    }

    public class HttpDefs {

        public String _URL;
        public JSONObject jsonData;
        public String _Data;
        public onHttpRequestListener Response;
        public List<NameValuePair> params;

        // public onHttpRequestListener cashResponse = null;

        public HttpDefs(String URL, onHttpRequestListener callback,
                List<NameValuePair> params) {

            Init(URL, callback, params);
        }

        private void Init(String URL, onHttpRequestListener callback,
                List<NameValuePair> params) {

            this.Response = callback;
            this._URL = URL;
            this.params = params;

        }
    }

    public class HttpBackground implements Runnable {
        HttpDefs client;
        String error;

        public HttpBackground(HttpDefs client) {
            this.client = client;

        }

        @Override
        public void run() {
            StringBuilder content = new StringBuilder();
            JSONObject jsonObj = new JSONObject();

            DefaultHttpClient httpclient;
            HttpParams params = new BasicHttpParams();
            HttpConnectionParams.setConnectionTimeout(params,
                    CONNECTION_TIMEOUT);
            HttpConnectionParams.setSoTimeout(params, WAIT_RESPONSE_TIMEOUT);
            // HttpConnectionParams.setTcpNoDelay(null, true);
            HttpProtocolParams.setVersion(params, HttpVersion.HTTP_1_1);
            httpclient = new DefaultHttpClient(params);
            String paramString = URLEncodedUtils.format(this.client.params,
                    "utf-8");
            if (!paramString.contentEquals(""))
                this.client._URL = this.client._URL;// +"&"+paramString;
            HttpPost httppost = new HttpPost(this.client._URL);
            try {
                httppost.setEntity(new UrlEncodedFormEntity(this.client.params));
            } catch (UnsupportedEncodingException e1) {
                // TODO Auto-generated catch block
                e1.printStackTrace();
            }
            httppost.getParams().setBooleanParameter(
                    CoreProtocolPNames.USE_EXPECT_CONTINUE, false);

            try {
                HttpResponse response = httpclient.execute(httppost);
                BufferedReader reader = new BufferedReader(
                        new InputStreamReader(
                                response.getEntity().getContent(), "UTF-8"));
                String sResponse;

                while ((sResponse = reader.readLine()) != null) {
                    content.append(sResponse);
                }

            } catch (UnsupportedEncodingException e) {
                error = "{\"type\":\"error\",\"value\":\"exception\",\"text\":\""
                        + e.getMessage() + "\"}";
                content.append(error);
                e.printStackTrace();
            } catch (ClientProtocolException e) {
                error = "{\"type\":\"error\",\"value\":\"exception\",\"text\":\""
                        + e.getMessage() + "\"}";
                content.append(error);
            } catch (IOException e) {
                error = "{\"type\":\"error\",\"value\":\"timeout\",\"text\":\"Connection Time out\"}";
                content.append(error);
            } finally {
                try {
                    if (content == null) {

                        jsonObj = new JSONObject(
                                "{\"type\":\"error\",\"value\":\"timeout\",\"text\":\"Connection Time out\"}");
                    } else if (content.toString().contentEquals("")) {
                        jsonObj = new JSONObject(
                                "{\"type\":\"error\",\"value\":\"timeout\",\"text\":\"Connection Time out\"}");
                    } else
                        jsonObj = new JSONObject(content.toString());

                } catch (JSONException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }

            }
            httpUI display = new httpUI(jsonObj, this.client);
            handler.post(display);

        }

    }

    class httpUI implements Runnable {

        HttpDefs client;
        String result = " ";
        JSONObject JsonResult;

        public httpUI(JSONObject res, HttpDefs client) {
            this.JsonResult = res;
            this.client = client;
        }

        public httpUI(String res, HttpDefs client) {
            this.result = res;
            this.client = client;
        }

        public void run() {
            HTTPResponse res = new HTTPResponse();
            try {
                // JSONObject JsonResult = new JSONObject(this.result);
                String type = "";
                if (JsonResult.has(AppStaticMembers.TAG_status))
                    type = JsonResult.getString(AppStaticMembers.TAG_status);

                if (type.contentEquals("true")) {
                    //
                    if (JsonResult.has("data"))
                        res.data = new JSONTokener(JsonResult.getString("data"))
                                .nextValue();
                    res.response = ResponseType.SUCESS;
                    if (JsonResult.has("message")) {
                        res.value = JsonResult.getString("message");
                    }

                } else {
                    res.response = ResponseType.ERROR;
                }

                if (JsonResult.has("message")) {
                    res.value = JsonResult.getString("message");
                }

                if (client.Response != null) {
                    client.Response.onHttpResult(res, "");

                }

                System.gc();
            } catch (Exception e) {
                e.printStackTrace();
                if (this.client.Response != null) {
                    this.client.Response.onHttpResult(res, "");
                }
            }
        }
    }

}

How I call it:

public void SignUp(userdata user, onHttpRequestListener callback) {

        String url = AppStaticMembers.GeneralURL + "Act=" + "signup";
        List<NameValuePair> params = new ArrayList<NameValuePair>();
        params.add(new BasicNameValuePair(AppStaticMembers.TAG_useremail,
                user.email));
        params.add(new BasicNameValuePair(AppStaticMembers.TAG_username,
                user.username));

        params.add(new BasicNameValuePair(AppStaticMembers.TAG_userpassword,
                user.password));
        params.add(new BasicNameValuePair("mob", user.mob));
        params.add(new BasicNameValuePair(AppStaticMembers.TAG_age, user.age));
        params.add(new BasicNameValuePair("regestration", user.regID));
        HttpDefs httpdefs = new HttpDefs(url, callback, params);
        executorService.submit(new HttpBackground(httpdefs));

    }

I would appreciate it if anyone could review my code.

Answer 1

As you don't mention anything in particular that you would like people to comment about, I'll make some general remarks regarding the style.

1) Is the name ConnectionExector on purpose or just a typo? If you want to shorten the name, it's not a good idea to remove just one letter, just rename it to ConnectionExecutor. Check your spelling elsewhere also (e.g. cashed)

2)

public enum ResponseType {
    SUCESS, ERROR, TIMEOUT, NULL;
}

This might be personal taste, but I would not name anything NULL in Java, as it might suggest its value is just null and confuse others who use your code. You could just use the Java null value.

    public ResponseType response = null;

Or just rename it to something like NONE or INIT.

3)

public ConnectionExector(Context context) {
    try {
        this.context = context;
        executorService = Executors.newFixedThreadPool(3);
    } catch (Exception e) {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
}

What's the purpose of capturing an exception there? I can see no cases where you would get one, and even if you get one you want the constructor to fail, as the only failure point would be the line

        executorService = Executors.newFixedThreadPool(3);

and if that fails, then your whole instance would be good for nothing.

4)

Use Java naming conventions:

    private void Init(String URL, onHttpRequestListener callback,

Methods should be verbs, in mixed case with the first letter lowercase, with the first letter of each internal word capitalized.

class httpUI implements Runnable {

Class names should be nouns, in mixed case with the first letter of each internal word capitalized.

5)

This looks like Android code, so use Log instead of printStackTrace when you catch an Exception.