2
\$\begingroup\$

I have a spinner in my activity that upon selection of one of it's options needs to handle the addition of a new object to my Sqlite database. the following solution works but everything in it looks like one big hack.
My layout:

<Spinner
    android:id="@+id/spnMachineManufacturer"
    android:layout_width="@dimen/et_params_width"
    android:layout_height="wrap_content"
    android:prompt="@string/Select"/>

My activity code:

public class ManufacturerSelectionActivity extends Activity
{
    Spinner spnMachineManufacturer;

    List<Manufacturer> allManufacturers;

    @Override
    protected void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_manufacturer_selection);

        spnMachineManufacturer = (Spinner) findViewById(R.id.spnMachineManufacturer);

        addItemsToManufacturersSpinner();
        addListenerToManufacturersSpinner();
    }

    private void addItemsToManufacturersSpinner()
    {
        final DbConnector db = new DbConnector(this);

        //populate list from db utility
        allManufacturers = db.getAllManufacturers();
        if (allManufacturers != null && allManufacturers.size() > 0)
        {
            //hack - create a manufacturer object for add new option
            Manufacturer addNewManufacturer = new Manufacturer();
            addNewManufacturer.setId(-1);
            addNewManufacturer.setName("Add new...");
            allManufacturers.add(addNewManufacturer);

            //add items to spinner
            final ArrayAdapter adapter = new ArrayAdapter<Manufacturer>(this,
            android.R.layout.simple_spinner_dropdown_item, allManufacturers);
            adapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
            spnMachineManufacturer.setAdapter(adapter);
        }
    }

    private void addListenerToManufacturersSpinner()
    {
        spnMachineManufacturer.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener()
        {
            @Override
            public void onItemSelected(AdapterView<?> adapterView, View view, int i, long l)
            {
                Manufacturer m = (Manufacturer) adapterView.getItemAtPosition(i);
                if (m != null)
                {
                    int size = adapterView.getCount();

                    //check if selected item is "add new..."
                    if (size == i + 1)
                    {
                        //create a dialog to receive the new manufacturer name
                        AlertDialog.Builder builder = new AlertDialog.Builder(
                        ManufacturerSelectionActivity.this);
                        builder.setTitle("Add a manufacturer");

                        // Set up the input
                        final EditText input = new EditText(ManufacturerSelectionActivity.this);

                        input.setInputType(InputType.TYPE_CLASS_TEXT);
                        builder.setView(input);

                        // Set up the buttons
                        builder.setPositiveButton("OK", new DialogInterface.OnClickListener()
                        {
                            @Override
                            public void onClick(DialogInterface dialog, int which)
                            {
                                String name = input.getText().toString();

                                final DbConnector db = new DbConnector(
                                ManufacturerSelectionActivity.this);

                                //Create a new manufacturer object
                                Manufacturer mnf = new Manufacturer();
                                mnf.setName(name);

                                //add the new manufacturer to the database
                                db.addManufacturer(mnf);

                                //refresh spinner
                                addItemsToManufacturersSpinner();
                            }
                        });
                        builder.setNegativeButton("Cancel", new DialogInterface.OnClickListener()
                        {
                            @Override
                            public void onClick(DialogInterface dialog, int which)
                            {
                                //do nothing
                                dialog.cancel();
                            }
                        });

                        builder.show();
                    }
                }
            }

            @Override
            public void onNothingSelected(AdapterView<?> adapterView)
            {
                //do nothing
            }
        });
    }
}

My concerns:

  • The addition of a manufacturer object to my list of manufacturers
  • The use of ManufacturerSelectionActivity.this as a context
  • The creation of a new item in the database from within the dialog method (which is found inside a setOnItemSelectedListener method
  • The call to addItemsToManufacturersSpinner() method from within the dialog method
\$\endgroup\$
2
\$\begingroup\$

Your method addListenerToManufacturersSpinner() is very long. You are nesting two listeners here. It would be easier to read if you defined the listener first and then set it.

For example:

DialogInterface.OnClickListener myOnClickListener = new DialogInterface.OnClickListener()
{
    @Override
    public void onClick(DialogInterface dialog, int which)
    {
        String name = input.getText().toString();

        final DbConnector db = new DbConnector(
        ManufacturerSelectionActivity.this);

        //Create a new manufacturer object
        Manufacturer mnf = new Manufacturer();
        mnf.setName(name);

        //add the new manufacturer to the database
        db.addManufacturer(mnf);

        //refresh spinner
        addItemsToManufacturersSpinner();
    }
});

And then you can set:

builder.setPositiveButton("OK", myOnClickListener);

Another point is, that you should define string resources such as "OK" in your res/values/strings.xml and reference the string there. This makes your app more configurable and separates content from code.

Variable naming:

 public void onItemSelected(AdapterView<?> adapterView, View view, int i, long l)

What do variables i and l do? It is good practie to use more descriptive names.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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