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

Fetch details from server

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

Problem

I am using this code to fetch some details from server from my android app. How can I improve it?

Here I am also transferring some data from one activity to another.

This is how I have written my Fetch_Details class:

```
public class Fetch_Details extends Activity{

//JSON Node Names
private static final String TAG_RESULT = "result";
private static final String TAG_AID = "id";
private static final String TAG_PID = "id";

String sessionId;
String subject;
String college;
String stage;
String addressedto;
String address;
String city;
String state;
String aid;
String pid;
ArrayList products = new ArrayList();
ArrayList productids = new ArrayList();
ArrayList quantities = new ArrayList();
int count;

@Override
public void onCreate(Bundle savedInstanceState)
{
super.onCreate(savedInstanceState);
setContentView(R.layout.fetch_details);

Intent intent = getIntent();
sessionId = intent.getStringExtra("sessionId");
subject = intent.getStringExtra("subject");
college = intent.getStringExtra("college");
stage = intent.getStringExtra("stage");
addressedto = intent.getStringExtra("addressedto");
address = intent.getStringExtra("address");
products = intent.getStringArrayListExtra("products");
quantities = intent.getStringArrayListExtra("quantities");

System.out.println(products);
System.out.println(quantities);

// count = products.size();
System.out.println("Size of ArrayList:- "+count);

if (android.os.Build.VERSION.SDK_INT > 9) {
StrictMode.ThreadPolicy policy = new StrictMode.ThreadPolicy.Builder().permitAll().build();
StrictMode.setThreadPolicy(policy);
}

new AsyncTask() {

private ProgressDialog dialog = new ProgressDialog(Fetch_Details.this);
@Override
protected void onPreExecute() {

Solution

These are some quick thoughts skipping through the code. Nothing android-specific, only regarding Java.

Fetch_Details breaks Java conventions. Use camelCase!

14 fields in a class is too much and is just begging to be split into several classes. For example, extract a class for adress information.

When possible, prefer using interfaces as types over implementations of interfaces.

ArrayList products = new ArrayList();
ArrayList productids = new ArrayList();
ArrayList quantities = new ArrayList();


becomes

List products = new ArrayList();
List productids = new ArrayList();
List quantities = new ArrayList();


productids should probably also be spelled productIds.

If you are using a newer Java version, you can save yourself generics in type declarations:

List products = new ArrayList<>();


Remove all those // TODO Auto-generated method stub comments. Remove them the second you write any code in that method. If you need a TODO there, write your own.

Remove System.out.println calls and commented-out code. Code that is commented out is dead code and printing to stdout is almost never useful. Prefer actual logging instead.

Comments like // Creating a new JSON Parser are utterly useless. If someone can't infer that from the code, they have never seen a line of Java in their life.

In general, if you want to write a comment, stop and really think about whether there isn't a better way to make the code comment itself. There pretty much almost always is a way. Comments are inherently bad in the sense that they will never improve, but only get worse and eventually start lying.

String accountquery = "Select id from Accounts Where accountname='"+college+"';";


This smells like SQL injections. Also in other places.

The formatting in the entire file is off. Formatting is very important. Code that is not formatted properly is automatically bad in my opinion. Use any IDE like Eclipse or IntelliJ and it will do formatting for you. Let the IDE do it's job!

e.printStackTrace();


Don't print stacktraces to stdout. Use proper logging, maybe rethrow exceptions. Think about whether ignoring exceptions like this is really a good way to go here.

Code Snippets

ArrayList<String> products = new ArrayList<String>();
ArrayList<String> productids = new ArrayList<String>();
ArrayList<String> quantities = new ArrayList<String>();
List<String> products = new ArrayList<String>();
List<String> productids = new ArrayList<String>();
List<String> quantities = new ArrayList<String>();
List<String> products = new ArrayList<>();
String accountquery = "Select id from Accounts Where accountname='"+college+"';";
e.printStackTrace();

Context

StackExchange Code Review Q#59889, answer score: 3

Revisions (0)

No revisions yet.