patternjavaMinor
Android Thread and Runnable
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
```
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
-
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
-
The correct spelling is
-
-
-
-
I totally agree with Marco that you should use
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.