Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Hallo,

I have written a simple spinner wrapper, but was wondering if any of you experts out there could think of any ways to make it more robust. It only handles strings at the moment, would you want anything else?

Anyway, the code for the (shamefully badly named) MySpinner class is:

package a.b.c;

import android.content.Context;
import android.util.AttributeSet;
import android.widget.ArrayAdapter;
import android.widget.Spinner;

public class MySpinner extends Spinner {

    // constructors (each calls initialise)
    public MySpinner(Context context) {
        super(context);
        this.initialise();
    }
    public MySpinner(Context context, AttributeSet attrs) {
        super(context, attrs);
        this.initialise();
    }
    public MySpinner(Context context, AttributeSet attrs, int defStyle) {
        super(context, attrs, defStyle);
        this.initialise();
    }

    // declare object to hold data values
    private ArrayAdapter<String> arrayAdapter;

    // add the selected item to the end of the list
    public void addItem(String item) {
        this.addItem(item, true);
    }
    public void addItem(String item, boolean select) {
        arrayAdapter.add(item);
        this.setEnabled(true);
        if (select) this.selectItem(item);
        arrayAdapter.sort(new Comparator<String>() {
            public int compare(String object1, String object2) {
                return object1.compareTo(object2);
            };
        });
    }

    // remove all items from the list and disable it
    public void clearItems() {
        arrayAdapter.clear();
        this.setEnabled(false);
    }

    // make the specified item selected (returns false if item not in the list)
    public boolean selectItem(String item) {
        boolean found = false;
        for (int i = 0; i < this.getCount(); i++) {
            if (arrayAdapter.getItem(i) == item) {
                this.setSelection(i);
                found = true;
                break;
            }
        }
        return found;
    }

    // return the current selected item
    public String getSelected() {
        if (this.getCount() > 0) {
            return arrayAdapter.getItem(super.getSelectedItemPosition());
        } else {
            return "";
        }
    }

    // allow the caller to use a different DropDownView, defaults to android.R.layout.simple_dropdown_item_1line
    public void setDropDownViewResource(int resource) {
        arrayAdapter.setDropDownViewResource(resource);
    }
    // internal routine to set up the array adapter, bind it to the spinner and disable it as it is empty
    private void initialise() {
        arrayAdapter = new ArrayAdapter<String>(super.getContext(), android.R.layout.simple_spinner_item);
        arrayAdapter.setDropDownViewResource(android.R.layout.simple_dropdown_item_1line);
        this.setAdapter(arrayAdapter);
        this.setEnabled(false);
    }
}

To use:

1. Use a.b.c.MySpinner instead of Spinner in your XML layout file
2. Set up a variable, mMySpinner = (MySpinner)findViewById(R.id.spinner);
3. You can then use all the functions which should be self-explanatory
4. If there are no items in the list, the spinner is disabled to prevent untoward events

mMySpinner.clearItems()      to remove all the items
mMySpinner.addItem("Blue")   to add Blue as an item in list (items are sorted by abc)
mMySpinner.selectItem("Red") to make the indicate item the current selection
mMySpinner.getSelected()     to return the current selected item string
share|improve this question
    
Why were so many common things like "add" and "clear" and "insert" and "delete" totally left out of Android Spinners, in the first place???? –  user9589 Dec 31 '11 at 6:02
add comment

3 Answers 3

This is a very nice class, and I don't wish to discredit the author (FrinkTheBrave) in any way! I do, however, want to offer a few enhancements and correct the nasty bugs that are inherent in the original.

The issues/enhancements addressed here:

o Do not insert duplicates

o Re-do selection if new row inserted

o getSelected will return null rather than empty string if nothing selected

o Proper comparison of string (a.equals(b) rather than a == b)

o Sort is now case-insensitive

o Removed 'this.' from this.foo where unneeded (which means, everywhere).

o Changed setDropDownViewResource as per comment by @seand

To explain more about the re-selection: Consider the following. Spinner currently has these items (where > indicates the selected item)

    Antelope
  > Cat
    Dog

Now, we insert Bear. Unless we re-do it, the second item will remain as 'selected.' But because we inserted Bear alphabetically, the true Selected item has moved to the third position.

    Antelope
  > Bear
    Cat
    Dog

So, before the insertion of an non-selected String, we will save the String of the current selection (if there is one). Then, after the insertion and the sort, we will select it again.

    Antelope
    Bear
  > Cat
    Dog  

So if I may be so bold....

package c.b.a;

import java.util.Comparator;

import android.content.Context;
import android.util.AttributeSet;
import android.widget.ArrayAdapter;
import android.widget.Spinner;

public class AdvancedSpinner extends Spinner {

    // Thanks to CodeReview: http://codereview.stackexchange.com/questions/491/simplified-android-spinner
    // declare object to hold data values
    private ArrayAdapter<String> arrayAdapter;

    // constructors (each call initialize)
    public AdvancedSpinner(Context context) {
        super(context);
        initialize();
    }

    public AdvancedSpinner(Context context, AttributeSet attrs) {
        super(context, attrs);
        initialize();
    }

    public AdvancedSpinner(Context context, AttributeSet attrs, int defStyle) {
        super(context, attrs, defStyle);
        initialize();
    }

    // add the selected item to the end of the list
    public Boolean addItem(String item) {
        return addItem(item, true);
    }

    public Boolean addItem(String item, boolean select) {
        Boolean addFlag = true;

        for (int i = 0; i < getCount(); i++) {
            if (arrayAdapter.getItem(i).equals(item)) {
                // Don't add an item that's already in the list
                // But we do need to mark it as "selected" if
                // applicable.
                addFlag = false;
                if (select)
                    setSelection(i);
                break;
            }
        }
        if (addFlag) {
            String saveSelected = null;
            if (! select) {
                // We need to preserve the prior selection, which may be
                // messed up by our array sort operation.  So we will
                // save the selected item's value, and (after the insert)
                // we will reinstate its selection status.
                saveSelected = getSelected();
            }
            arrayAdapter.add(item);
            setEnabled(true);
            arrayAdapter.sort(new Comparator<String>() {
                public int compare(String object1, String object2) {
                    return  object1.compareToIgnoreCase(object2);
                };
            });
            if (select)
                selectItem(item);
            else if (saveSelected != null)
                selectItem(saveSelected);
        }
        return addFlag;
    }

    // remove all items from the list and disable it
    public void clearItems() {
        arrayAdapter.clear();
        setEnabled(false);
    }

    // make the specified item selected (returns false if item not in the list)
    public boolean selectItem(String item) {
        boolean found = false;
        for (int i = 0; i < getCount(); i++) {
            if (arrayAdapter.getItem(i).equals(item)) {
                setSelection(i);
                found = true;
                break;
            }
        }
        return found;
    }

    public String getSelected() {
        // return the current selected item
        // Changed (Dennis) to return null if nothing selected, rather than the
        // (misleading) empty string.  Also, prior incarnation would ForceClose
        // if array was not empty but nothing was selected.
        String rtnVal = null;
        if (getCount() > 0) {
            int i = super.getSelectedItemPosition();
            if (i >= 0)  // An item has been selected
                rtnVal = arrayAdapter.getItem(i);
        }
        return rtnVal;
    }

    // allow the caller to use a different DropDownView, defaults to android.R.layout.simple_spinner_dropdown_item
    public void setDropDownViewResource(int resource) {
        arrayAdapter.setDropDownViewResource(resource);
    }

    // internal routine to set up the array adapter, bind it to the spinner and disable it as it is empty
    private void initialize() {
        arrayAdapter = new ArrayAdapter<String>(super.getContext(), android.R.layout.simple_spinner_item);
        arrayAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
        setAdapter(arrayAdapter);
        setEnabled(false);
    }
}
share|improve this answer
    
Welcome to CodeReview! It would be helpful, I think, if you could summarize the issues at the top of your post, as well as having them in comments, so that they're more obvious. –  Glenn Rogers Sep 25 '12 at 4:09
    
@GlennRogers is correct, of course. I hope my edits have done an adequate job of explaining what was done, and why. –  Dennis Sep 25 '12 at 8:53
    
@Dennis, thanks for your comments/corrections, that's why I posted it. However, I disagree mildly with preventing duplicates as that should be under the control of the calling program, and I disagree strongly with returning null if nothing selected as it makes it harder to call. How would you call getSelected if it could return a null? –  FrinkTheBrave Oct 8 '12 at 12:16
    
@FrinkTheBrave, that's why they make chocolate and vanilla. But it is a standard to return null for non-existent. How would I call it? If ((myString = getSelected()) != NULL) { // do something with myString } If that's harder to call, I apologize. But I'd be willing to bet you're going to do something like <<if (myString != "")>> in your code, and I see zero difference in ease or in readability. –  Dennis Oct 10 '12 at 0:40
    
@Dennis, I was more thinking that 9 times out of 10 I'd be just assigning the spinner value to a string as data, myString = getSelected(); rather than acting on its value at the time. Also, I'm sure ((myString = getSelected()) != NULL) makes sense to all those C and Java wizards out there but it just looks complex to me. Thanks for your comments tho :-) –  FrinkTheBrave Oct 13 '12 at 20:34
show 1 more comment

A few ways you could make it more robust.

  1. In getSelected(), why not call getSelectedItem() instead?
  2. You tend to use super.foo() instead of foo(); that's usually a bad idea. Just call foo(), unless you really want to avoid any override in your class. If you always use super.foo(), then if you decide you want to override foo(), perhaps for some debug code, you have to go change your code everwhere from super.foo() to just foo() or perhaps this.foo().
  3. This code doesn't seem to enforce a consistent policy around duplicates in the spinner. If duplicates are not allowed, perhaps good to check for that at add time.

My €0.02.

share|improve this answer
add comment

Thanks for this great Spinner class. It saved my day, actually two :)

However, I think it would be great to replace:

arrayAdapter.setDropDownViewResource(android.R.layout.simple_dropdown_item_1line);

with:

arrayAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);

My issue was to implement two spinners with the following feature: if some string is selected in the spinner 1, it should disappear in spinner 2 and the same for the second spinner.

So it would be very nice to have some method to "hide" some item. I'm just clearing the adapter and adding other items, without hidden.

Really thank you for sharing such good things.

Update after half a day of workaround:

  1. UTLIMATE BUG: if(arrayAdapter.getItem(i) == item)
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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