2
\$\begingroup\$

The code below is used to download a JSON data from the web and use the data to fill a spinner. I would like to know if there is better way. Is it wise to move everything from onGetPriceType to the AsyncTask?

Interface

public interface GetBuildTypeInterface {
    public void onGetBuildType(HashMap<String,String> result);
}

AsyncTask

public class GetBuildType extends AsyncTask<Void, Void, JSONArray> {
    private GetBuildTypeInterface callback;

    public GetBuildType(GetBuildTypeInterface callback) {
        this.callback = callback;
    }

    @Override
    protected JSONArray doInBackground(Void... params) {
        UserFunctions u = new UserFunctions();
        return u.getBt();
    }

    @Override
    protected void onPostExecute(JSONArray result) {
        super.onPostExecute(result);
        HashMap<String, String> map = new HashMap<String, String>();

        if (result != null) {
            map = result.parse();
            callback.onGetBuildType(map);

        } else {
            callback.onGetBuildType(null);
        }
    }
}

Activity

public class AddFillActivityApp extends ErrorActivity implements
        GetBuildTypeInterface

{
    ArrayAdapter<String> adapterBuild;
    Spinner spinBuildType;

@Override
    public void onGetPriceType(final HashMap<String, String> result) {
        if (result != null) {
            List<String> fields = new ArrayList<String>();
            fields.addAll(result.keySet());
            adapterPrice = new ArrayAdapter<String>(this,
                    android.R.layout.simple_spinner_item, fields);
            adapterPrice
                    .setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
            spinPriceType.setAdapter(adapterPrice);

            spinPriceType
                    .setOnItemSelectedListener(new OnItemSelectedListener() {
                        @Override
                        public void onItemSelected(AdapterView<?> parent,
                                View view, int position, long id) {

                            if (position > -1) {
                                String selection = spinPriceType
                                        .getSelectedItem().toString();
                                a.setNot_priceFor(result.get(selection));
                            }   
                        }

                        @Override
                        public void onNothingSelected(AdapterView<?> arg0) {    
                        }
                    });
        }else error();
    }
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I see a very big advantage of your current approach rather than moving the implementation of onGetPriceType to the GetBuildType class (May I suggest changing the name to GetBuildTypeTask?)

  • Decoupling code. Your GetBuildType class is 100% independent of your activity. That is a good sign. If needed, you could at any time re-use the GetBuildType class and provide it with another GetBuildTypeInterface.

A side suggestion: I assume that result.parse() doesn't need to be run on the UI-thread, so you can change your class to extend AsyncTask<Void, Void, Map<String, String>> and make the conversion to map in doInBackground (or return null if u.getBt() returns null). Also, you don't need to create a new empty HashMap before parsing the JsonArray, simply Map<String, String> map = jsonArray.parse(); will do.

The only disadvantage I see with your current implementation is that you get an extra interface. But I really don't think that's so bad compared to having decoupled code.

\$\endgroup\$

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.