4
\$\begingroup\$

Please review this Android UnitConverter App which has been designed using Strategy Pattern.

public class UnitConverter extends Activity 
    implements OnClickListener, AdapterView.OnItemSelectedListener {
    /** Called when the activity is first created. */

    private Spinner SpinnerUnit;
    private EditText inputValue;
    private Spinner SpinnerFrom;
    private Spinner SpinnerTo;
    private Button ButtonConvert;
    private EditText ResultView;
    ArrayAdapter<String> unitarray;
    ArrayAdapter<String> unitarrayadapter;
    private Strategy currentStrategy;
    private String unitfrom;
    private String unitto;
    private static UnitConverter instance;

    //this is to test the Git repository
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);

        SpinnerUnit = (Spinner)findViewById(R.id.SpinnerUnit);
        SpinnerUnit.setOnItemSelectedListener(this);

        unitarray=new ArrayAdapter<String>(this,android.R.layout.simple_spinner_item);
        unitarray.
            setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
        SpinnerUnit.setAdapter(unitarray);
        unitarray.add(getResources().getString(R.string.unit1));
        unitarray.add(getResources().getString(R.string.unit2));
        unitarray.add(getResources().getString(R.string.unit3));
        unitarray.add(getResources().getString(R.string.unit4));
        unitarray.add(getResources().getString(R.string.unit5));
        unitarray.add(getResources().getString(R.string.unit6));
        unitarray.add(getResources().getString(R.string.unit7));
        unitarray.add(getResources().getString(R.string.unit8));
        unitarray.setNotifyOnChange(true);

        SpinnerFrom = (Spinner)findViewById(R.id.SpinnerFrom);
        SpinnerFrom.setOnItemSelectedListener(this);
        SpinnerTo = (Spinner)findViewById(R.id.SpinnerTo);
        SpinnerTo.setOnItemSelectedListener(this);

        unitarrayadapter = 
            new ArrayAdapter<String>(this,android.R.layout.simple_spinner_item); 
        unitarrayadapter.
            setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
        SpinnerFrom.setAdapter(unitarrayadapter);
        SpinnerTo.setAdapter(unitarrayadapter);

        unitarrayadapter.setNotifyOnChange(true);

        ResultView = (EditText)findViewById(R.id.TextViewResult);
        ResultView.setClickable(false);

        ButtonConvert = (Button)findViewById(R.id.Button01);

        ButtonConvert.setOnClickListener(this);

        inputValue = (EditText)findViewById(R.id.EditTextValue);

        //initialization
       currentStrategy = new TemperatureStrategy();

       instance = this;

    }

    public static UnitConverter getInstance(){
        return instance;
    }

    public void onItemSelected(AdapterView<?> parent){  
    }

    public void onNothingSelected(AdapterView<?> parent){

    }

    public void onItemSelected(AdapterView<?> parent, View v,
            int position, long id){

        if(v.getParent() == SpinnerUnit){

                switch(position){
                case 0:
                    setStrategy(new TemperatureStrategy());
                    break;

                case 1:
                    setStrategy( new WeightStrategy());
                    break;

                case 2:
                    setStrategy(new LengthStrategy());
                    break;

                case 3:
                    setStrategy(new PowerStrategy());
                    break;

                case 4:
                    setStrategy(new EnergyStrategy());
                    break;

                case 5:
                    setStrategy(new VelocityStrategy());
                    break;

                case 6:
                    setStrategy(new AreaStrategy());
                    break;

                case 7:
                    setStrategy(new VolumeStrategy());
                    break;
                }


                fillFromToSpinner(position);

                SpinnerFrom.setSelection(0);
                SpinnerTo.setSelection(0);

                //If only first spinner is selected and
                //the from and to spinners are not clicked at all
                unitfrom = (String)(SpinnerFrom.getItemAtPosition(0).toString());
                unitto = (String)(SpinnerTo.getItemAtPosition(0).toString());

                //reset the result
                ResultView.setText("");


        }
        else if(v.getParent() == SpinnerFrom){
            unitfrom = (String)(SpinnerFrom.getSelectedItem().toString());  
        }

        else if(v.getParent() == SpinnerTo){
            unitto = (String)(SpinnerTo.getSelectedItem().toString());
        }
    }   


    private void fillFromToSpinner(int position){

        switch(position)
        {
        case 0:
            fillSpinnerWithTempUnit();
            break;

        case 1:
            fillSpinnerWithWeightUnit();
            break;

        case 2:
            fillSpinnerWithLengthUnit();
            break;

        case 3:
            fillSpinnerWithPowerUnit();
            break;

        case 4:
            fillSpinnerWithenErgyUnit();
            break;

        case 5:
            fillSpinnerWithVelocityUnit();
            break;

        case 6:
            fillSpinnerWithAreaUnit();
            break;

        case 7:
            fillSpinnerWithVolumeUnit();
            break;
        }

    }

    private void fillSpinnerWithTempUnit(){
        unitarrayadapter.clear();
        unitarrayadapter.add(getResources().getString(R.string.temperatureunitc));
        unitarrayadapter.add(getResources().getString(R.string.temperatureunitf));
        unitarrayadapter.notifyDataSetChanged();
    }

    private void fillSpinnerWithWeightUnit(){
        unitarrayadapter.clear();
        unitarrayadapter.add(getResources().getString(R.string.weightunitkg));
        unitarrayadapter.add(getResources().getString(R.string.weightunitgm));
        unitarrayadapter.add(getResources().getString(R.string.weightunitlb));
        unitarrayadapter.add(getResources().getString(R.string.weightunitounce));
        unitarrayadapter.add(getResources().getString(R.string.weightunitmg));
        unitarrayadapter.notifyDataSetChanged();
    }

    private void fillSpinnerWithLengthUnit(){
        unitarrayadapter.clear();
        unitarrayadapter.add(getResources().getString(R.string.lengthunitmile));
        unitarrayadapter.add(getResources().getString(R.string.lengthunitkm));
        unitarrayadapter.add(getResources().getString(R.string.lengthunitm));
        unitarrayadapter.add(getResources().getString(R.string.lengthunitcm));
        unitarrayadapter.add(getResources().getString(R.string.lengthunitmm));
        unitarrayadapter.add(getResources().getString(R.string.lengthunitinch));
        unitarrayadapter.add(getResources().getString(R.string.lengthunitfeet));
    }

    private void fillSpinnerWithPowerUnit(){
        unitarrayadapter.clear();
        unitarrayadapter.add(getResources().getString(R.string.powerunitwatts));
        unitarrayadapter.add(getResources().getString(R.string.powerunithorseposer));
        unitarrayadapter.add(getResources().getString(R.string.powerunitkilowatts));
    }

    private void fillSpinnerWithenErgyUnit(){
        unitarrayadapter.clear();
        unitarrayadapter.add(getResources().getString(R.string.energyunitcalories));
        unitarrayadapter.add(getResources().getString(R.string.energyunitjoules));
        unitarrayadapter.add(getResources().
            getString(R.string.energyunitkilocalories));

    }

    private void fillSpinnerWithVelocityUnit(){
        unitarrayadapter.clear();
        unitarrayadapter.add(getResources().getString(R.string.velocityunitkmph));
        unitarrayadapter.add(getResources().getString(R.string.velocityunitmilesperh));
        unitarrayadapter.add(getResources().getString(R.string.velocityunitmeterpers));
        unitarrayadapter.add(getResources().getString(R.string.velocityunitfeetpers));
    }

    private void fillSpinnerWithAreaUnit(){
        unitarrayadapter.clear();
        unitarrayadapter.add(getResources().getString(R.string.areaunitsqkm));
        unitarrayadapter.add(getResources().getString(R.string.areaunitsqmiles));
        unitarrayadapter.add(getResources().getString(R.string.areaunitsqm));
        unitarrayadapter.add(getResources().getString(R.string.areaunitsqcm));
        unitarrayadapter.add(getResources().getString(R.string.areaunitsqmm));
        unitarrayadapter.add(getResources().getString(R.string.areaunitsqyard));    
    }

    private void fillSpinnerWithVolumeUnit(){
        unitarrayadapter.clear();
        unitarrayadapter.add(getResources().getString(R.string.volumeunitlitres));
        unitarrayadapter.add(getResources().getString(R.string.volumeunitmillilitres));
        unitarrayadapter.add(getResources().getString(R.string.volumeunitcubicm));
        unitarrayadapter.add(getResources().getString(R.string.volumeunitcubiccm));
        unitarrayadapter.add(getResources().getString(R.string.volumeunitcubicmm));
        unitarrayadapter.add(getResources().getString(R.string.volumeunitcubicfeet));
    }

    public void onClick(View v){
        if(v == ButtonConvert){
            if(!inputValue.getText().toString().equals("")){
                double in = Double.parseDouble(inputValue.getText().toString());
                double result = currentStrategy.Convert(unitfrom, unitto, in);
                ResultView.setText(Double.toString(result));
            }
            else {
                ResultView.setText("");
            }
        }
    }

    private void setStrategy(Strategy s){
        currentStrategy = s;
    }        
}

The Strategy interface:

package training.android.trainingunitconverter;

public interface Strategy {

    public double Convert(String from, String to, double input);
}

And the different strategies for different Units like the following:

package training.android.trainingunitconverter;

public class TemperatureStrategy implements Strategy {

    public double Convert(String from, String to, double input) {
        // TODO Auto-generated method stub

        if((from.equals(
            UnitConverter.getInstance().getApplicationContext().getResources().
                getString(R.string.temperatureunitc)) && 
            to.equals((UnitConverter.getInstance().getApplicationContext().
                getResources().getString(R.string.temperatureunitf))))){
            double ret = (double)((input*9/5)+32);
            return ret;
        }

        if((from.equals(UnitConverter.getInstance().getApplicationContext().
            getResources().getString(R.string.temperatureunitf)) && 
            to.equals((UnitConverter.getInstance().getApplicationContext().
                getResources().getString(R.string.temperatureunitc))))){
            double ret = (double)((input-32)*5/9);
            return ret;
        }

    if(from.equals(to)){
        return input;   
    }
    return 0.0;
    }
}

Similarly for other strategies...

The source code can also be downloaded from here.

\$\endgroup\$
1
  • \$\begingroup\$ Never am able to point this out on SO, so I am so happy to finally be able to mention this: Whenever Eclipse creates methods, delete the auto-generated method stub comment. It has no place of being there whatsoever! \$\endgroup\$
    – Ingo Bürk
    May 3, 2014 at 21:08

1 Answer 1

1
\$\begingroup\$

As the comment says, you should post the code, possibly in smaller parts. Until then some general suggestions:

  • Clean up the indentation. You are mixing spaces and tabs.
  • Clean up the variable/method names. By convention Java variables and method names start with a small letter and use camel case. For example, fillspinnerwithtempunit is difficult to read. fillSpinnerWithTempUnit would be better.
  • Speaking of which: All the fillspinnerwith... methods are unnecessary. Add a method to your Strategy interface and get the list of units from the Strategy implementations. That will also get rid of the the second of the two switchs, which both switch over exactly the same values.
  • The first switch should be replaced by an Array of class references.
  • All the repetitive ...add(getResources().getString(... blocks should be loops instead.
  • You have all the units string in the resource file, but they are hard coded in the Strategies, too. If the resources are ever translated (that's why they are resources in the first place), then your app will break.

There are many other problems with your code, but this should be a good start.

\$\endgroup\$
4
  • \$\begingroup\$ github repos at github.com/sommukhopadhyay/UnitConverter has been updated... the app has been completely refactored for multilingual resources... \$\endgroup\$ Jun 18, 2011 at 8:04
  • \$\begingroup\$ Work on the other points - and move the Strategies into separate file again. It's completely unusable to have everything in one file. \$\endgroup\$
    – RoToRa
    Jun 18, 2011 at 12:38
  • \$\begingroup\$ RoToRa, suggest me a way to do that... \$\endgroup\$ Jun 19, 2011 at 13:58
  • \$\begingroup\$ Hi RoToRa, thank you for your valuable input... i have re-factored it again... this time the strategy class is not an inner class of the main Activity... instead i am maintaining a static reference of the instance of the main activity class throughout the package... and through this reference i am accessing the resources... and its working perfectly... \$\endgroup\$ Jul 11, 2011 at 12:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.