Three edittext boxes: suggestions to clean up code

Question

I am a newbie at writing Android apps and using Java in general. I have went through most of the Android hello view tutorials but still seem to be lacking some understanding of the basics. Here is a snippet of code I wrote for an app. My goal was to have three edittext boxes for the user to input information. I want the user to put information in two of the three boxes and when they hit the calculate button, it will calculate the third edittext value based on a specific equation. Is there anyway that I can avoid initializing these 3 edittext objects in each of my methods within this class? Any other suggestions for cleaning up or improving this code?

public class PlantPopulation extends Activity {
 /** Called when the activity is first created. */ 
 @Override
 public void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 setContentView(R.layout.main);
 EditText seedSpacing = (EditText) 
 findViewById(R.id.seedSpacing);
 EditText rowSpacing = (EditText) 
 findViewById(R.id.rowSpacing);
 EditText population = (EditText) 
 findViewById(R.id.population);
 seedSpacing.setText(""); //clear values
 rowSpacing.setText("");
 population.setText("");
 }
 //check how many boxes are empty
 public int checkifempty(String array[]) {
 int i = 0;
 for (String string : array) {
 if (string.equals("")) {
 i = i+1;
 } 
 }
 return i;
 }
 //run the calculation
 public void calc(View v) { 
 EditText seedSpacing = (EditText) 
 findViewById(R.id.seedSpacing);
 EditText rowSpacing = (EditText) 
 findViewById(R.id.rowSpacing);
 EditText population = (EditText) 
 findViewById(R.id.population);
 String sS = seedSpacing.getText().toString();
 String rS = rowSpacing.getText().toString();
 String pop = population.getText().toString();
 String boxes[] = {sS,rS,pop}; 
 //determine which box is empty
 if (checkifempty(boxes) <2) {
 if (sS.equals("")) {
 double calc1=(43560*144)/new Double(rS)/
 new Double(pop);
 calc1 = Math.floor(calc1 * 100 +.5)/100;
 seedSpacing.setText(Double.toString(calc1));
 } else if (rS.equals("")) {
 double calc2=(43560*144)/new Double(sS)/
 new Double(pop);
 calc2 = Math.round(calc2);
 rowSpacing.setText(Double.toString(calc2));
 } else if (pop.equals("")) {
 Double calc3=((43560*144)/new Double(rS)/
 new Double(sS));
 Integer calc = calc3.intValue();
 //calc3 = Math.round(calc3);
 population.setText(calc.toString());
 } else {
 Toast.makeText(PlantPopulation.this, 
 "Leave one item blank.", Toast.LENGTH_SHORT).show();
 } 
 } else {
 Toast.makeText(PlantPopulation.this, 
 "You must fill in two of the three boxes.", Toast.LENGTH_SHORT).show();
 }
 }
}

• SeRPRo: One suggestion... use the camel-case to name your functions/variables as you did in you class, though I only put in capital the first character on classes.
• ObscureRobot: Better fit for CodeReview.

Answer 1

Declare and initialise the edit texts like this:

public class PlantPopulation extends Activity {
 EditText seedSpacing;
 EditText rowSpacing;
 EditText population;
/** Called when the activity is first created. */ 
 @Override
public void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 setContentView(R.layout.main);
 seedSpacing = (EditText) 
 findViewById(R.id.seedSpacing);
 rowSpacing = (EditText) 
 findViewById(R.id.rowSpacing);
 population = (EditText) 
 findViewById(R.id.population);
.....

You now will not need to re-declare and initialise them in other methods, namely your calc method in this example.

Hope that helps, Barry

• Rick: When I tried that, i get a null point exception and the program will not run on the emulator. Maybe I am missing something?
• barry: Hmm... where are you calling calc from?
• Rick: i am calling it with the xml attribute android:onClick="calc"
• Rick: I tried changing everything back to the way i had it and now can't get it to work. it seems like i keep getting the null point exception at the statement of seedSpacing.setText(""); or at any other edittext object. If i have this statement in the onCreate method, the program won't run. if i have this statement in either my calc or clear method, those crash the program when i click the corresponding button.
• barry: What do you mean by "the way you had it" and "this statement"?

Answer 2

• Replace "43560*144" by a constant.
• Store new Double(sS), new Double(srS), new Double(pop) in variables before if (checkifempty(boxes) <2). It'll improve speed and clarify the code.
• Correct indentation in the onCreate method

Answer 3

Replace horrible checkIfEmpty method, which by the way doesn't do what you plan it to do. Use addTextChangedListener for your EditTexts.
In your xml, set property android:text to an empty value, so you can avoid doing it in the code.

• Rick: the checkIfEmpty is providing me a way to see how many boxes are left empty by the user, to avoid any calculation errors if more than one box is empty or if non are empty. how would i implement the addTextChangedListener to accomplish this same task?

Answer 4

Some general (Java) idea without any Android specific stuff:

1, Extract the get*Spacing methods, for example:

publiv EditText getSeedSpacing() {
 return (EditText) findViewById(R.id.seedSpacing);
}

It removes some code duplication.

2, Consider using i++ instead of i = i+1.

3, Use longer names:

String seedSpacingValue = seedSpacing.getText().toString();
String rowSpacingValue = rowSpacing.getText().toString();
String populationValue = population.getText().toString();

It makes the code more readable.

2, Rename the checkifempty method. It does not check anything, it returns the count of empty fields. So for the current functionality it should be countEmptyFields or something like this. Anyway, I'd write the following:

public int countEmptyFields(final EditText... fields) {
 int count = 0;
 for (EditText field: fields) {
 final String value = field.getText();
 if ("".equals(value)) {
 count++
 } 
 }
 return count;
}
public void showError(final String msg) {
 Toast.makeText(PlantPopulation.this, msg, Toast.LENGTH_SHORT).show();
}

then in the calc method:

...
final int emptyFields = countEmptyFields(seedSpacing, rowSpacing, population); 
if (emptyFields == 0) {
 showError("Leave one item blank.");
 return;
}
if (emptyFields > 2) {
 showError("You must fill in two of the three boxes.");
 return;
}
if (seedSpacingValue.equals("")) ...
...

The showError removes some code duplication while the == 0 and > 2 checks at the beginning replaces the nested blocks to an easier to follow structure.

• Rick: thank you for the suggestions. they make sense and your explanations were very helpful

Answer 5

Use Andject to get rid of something = getViewById():

https://github.com/ko5tik/andject

( Shameles self-advertizing )

Speciofy desired views via annotations:

 class WithInjectableViews extends Activity {
 // shall be injected
 @InjectView(id = 239)
 private android.view.View asView;
 @InjectView(id = 555)
 private Button button;
 // shall be left alone
 private View notInjected = null;
}

And in your onCreate() say:

 ViewInjector.startActivity(injectable);

There is also work undeway to inject shared preference values ( also provides cleaner code and declarative style )