HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Android Thread and Runnable

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
andthreadrunnableandroid

Problem

This is my code for running a background thread. I believe it is very poor in naming and code structure.

```
package com.ocs.socialshare.bloggershare;

import android.app.Activity;
import android.os.Bundle;
import android.view.Menu;
import android.view.View;
import android.view.View.OnClickListener;
import android.widget.Button;
import android.widget.EditText;

import com.google.gdata.util.AuthenticationException;

public class MainActivity extends Activity implements OnClickListener{

private Thread doinBackground;

private EditText username,password,blogurl;
private Button login;
private String user,pass,bloglink;

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
findViewById();
setOnclickListenerRegister();
}

private void findViewById() {
// TODO Auto-generated method stub
username = (EditText) findViewById(R.id.username);
password = (EditText) findViewById(R.id.password);
blogurl = (EditText) findViewById(R.id.blogurl);
login = (Button) findViewById(R.id.login);
}

private void setOnclickListenerRegister() {
// TODO Auto-generated method stub
login.setOnClickListener(this);
}
private void blogShare(final String user,final String pass,final String bloglink) {
// TODO Auto-generated method stub

doinBackground =new Thread(new Runnable()
{
@Override
public void run()
{
// TODO Auto-generated method stub
BloggerClient blog = new BloggerClient(getApplicationContext());
try {
blog.oAuthBlogger(user,pass,bloglink);
}
catch (AuthenticationException e)
{
// TODO Auto-generated ca

Solution

-
Please remove all // TODO Auto-generated method stub comments. Your methods are implemented. You don't need those comments any more.

-
Indentation. If you're using Eclipse, select all code and press Ctrl + Shift + I to ident your code properly. If you're using NetBeans, press Alt + Shift + F. Your try-catch inside the blogShare method is incorrectly indented.

-
The correct spelling is validateField

-
user, pass and bloglink are only set within validateField and only read directly after that method invocation. Get rid of the validateField method (which currently doesn't do any actual validation so it's a poorly named method). Use local variables within the onClick method.

-
int viewid = view.getId(); is IMO not needed to set as a variable, as you only check it once, use view.getId() directly.

-
setOnclickListenerRegister is only called once from onCreate and this method only contains one line, so what you have done is to extract one statement to it's own method and then call that method which were only called once. This extraction is not worth it. Remove this method and put login.setOnClickListener(this); back to the onCreate method where it belongs.

-
I totally agree with Marco that you should use AsyncTask. If you encounter any problems with orientation changes, deal with those correctly. Don't avoid using AsyncTask just because you're afraid that it will cause problems.

Context

StackExchange Code Review Q#44125, answer score: 8

Revisions (0)

No revisions yet.