4
\$\begingroup\$

This is my adapter class, to create a custom listView with three radiobuttons:

public class Attendancelist_customadapter extends ArrayAdapter {
    ///needs two arraylists - one for the peopledetails, and one for the attendance status. read from file. passed to this.
    ///then assemble into a ListView with customview - in this.
    ///three radio buttons per item. invited, not attending, attending.
    ///
    Context context; 
    int layoutResourceId;    
    ArrayList<People_Attendance> people_attendance = null;
    String[] attendance_status;
    String ATTENDING ="ATTENDING";
    String NOT_ATTENDING = "NOT ATTENDING";
    String INVITED = "INVITED";
    public Attendancelist_customadapter(Context context, int layoutResourceId, ArrayList<People_Attendance> people_attendance){
        super(context, layoutResourceId, people_attendance);
        this.layoutResourceId = layoutResourceId;
        this.context = context;
        this.people_attendance = people_attendance;
        this.attendance_status = new String[this.people_attendance.size()];

    }
    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        View row = convertView;
        attendance_holder holder = null;
        People_Attendance people_array[] = people_attendance.toArray(new People_Attendance[people_attendance.size()]);
        if(row == null)
        {
            LayoutInflater inflater = ((Activity)context).getLayoutInflater();
            row = inflater.inflate(layoutResourceId, parent, false);

            holder = new attendance_holder();
            holder.txtName = (TextView)row.findViewById(R.id.name_txt);
            holder.txtNumber = (TextView)row.findViewById(R.id.number_txt);
            holder.attendance_group = row.findViewById(R.id.Attendance_Group);
            holder.not_attending = (RadioButton) holder.attendance_group.findViewById(R.id.not_attending_radio);
            holder.attending = (RadioButton) holder.attendance_group.findViewById(R.id.attending_radio);
            holder.invited = (RadioButton) holder.attendance_group.findViewById(R.id.invited_radio);


            row.setTag(holder);
        }
        else
        {
            holder = (attendance_holder)row.getTag();
        }


        holder.txtName.setText(people_array[position].name);
        holder.txtNumber.setText(people_array[position].number);
        holder.invited.setChecked(people_array[position].attendance_status.equals(INVITED));
        holder.attending.setChecked(people_array[position].attendance_status.equals(ATTENDING));
        holder.not_attending.setChecked(people_array[position].attendance_status.equals(NOT_ATTENDING));

        holder.attending.setTag(position);
        holder.not_attending.setTag(position);
        holder.invited.setTag(position);

        holder.invited.setOnCheckedChangeListener((new CompoundButton.OnCheckedChangeListener() {
            public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
                int position = (Integer) buttonView.getTag();
                if (isChecked == true){
                    attendance_status[position] = INVITED;
                }
            }
        }));
        holder.attending.setOnCheckedChangeListener((new CompoundButton.OnCheckedChangeListener(){
            public void onCheckedChanged(CompoundButton buttonView, boolean isChecked){
                int position = (Integer) buttonView.getTag();
                if (isChecked == true){
                    attendance_status[position] = ATTENDING;
                }
            }

        }));
        holder.not_attending.setOnCheckedChangeListener((new CompoundButton.OnCheckedChangeListener() {

            @Override
            public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
                // TODO Auto-generated method stub
                int position = (Integer) buttonView.getTag();
                if (isChecked == true){
                    attendance_status[position] = NOT_ATTENDING;
                }
            }
        }));

        return row;
        }
        People_Attendance getPerson(int position){
            return ((People_Attendance) getItem(position));
        }




    static class attendance_holder{
        TextView txtName;
        TextView txtNumber;
        RadioButton invited;
        RadioButton attending;
        RadioButton not_attending;
        View attendance_group;

        public String getChecked(){
            String return_string = "";
            if(invited.isChecked()){
                return_string = "INVITED";    
                return return_string;
            }else if(attending.isChecked()){
                return_string = "ATTENDING";
                return return_string;
            }else if(not_attending.isChecked()){
                return_string = "NOT ATTENDING";
                return return_string;
            }else{
                invited.setChecked(true);///checks the Invited checkbox, if none others are checked.
                /// as only people who have been invited will be passed to this, therefore set to invited if nothing else.
                return_string = "INVITED";
                return return_string;
            }

        }
    }
}
  1. Is there a shorter way of writing the code for the Listeners for each of the RadioButtons?
  2. Is there a suitable method that could be implemented from the RadioGroup, that would shorten the code?

In my main activity I will get the string[] attendance_status, and then use that to write to a file. My question is whether there is a shorter, more efficient code that I could use, instead of having three onCheckedChangeListners?

\$\endgroup\$
3
\$\begingroup\$

This is your lucky day, there is an "easier" way to do this. First, we create a custom inner class: (put this code inside your current class, just like you would do with a method)

private class MyCheckedChange implements CompoundButton.OnCheckedChangeListener {
    private final String status;
    public MyCheckedChange(String status) {
        this.status = status;
    }
    @Override
    public void onCheckedChanged(CompoundButton buttonView, boolean isChecked){
        int position = (Integer) buttonView.getTag();
        if (isChecked == true){
            attendance_status[position] = status;
        }
    }
}

Then you can instantiate this class with a corresponding status value.

holder.attending.setOnCheckedChangeListener(new MyCheckedChange(ATTENDING));
holder.not_attending.setOnCheckedChangeListener(new MyCheckedChange(NOT_ATTENDING));
holder.invited.setOnCheckedChangeListener(new MyCheckedChange(INVITED));

Further suggestions: (use them if you like them)

Use an enum for attendance status:

public enum AttendingStatus {
    NOT_ATTENDING, ATTENDING, INVITED;
}

And use AttendingStatus[] attendance_status; instead of your string array.

This will open up the possibility of many things, if you would like:

You can match up your radiobuttons to an attendingstatus, by using a Map<RadioButton, AttendingStatus> for example, which could reduce code duplication in the getChecked() method by grabbing a RadioButtons corresponding AttendingStatus by using AttendingStatus status = map.get(radioButton); and using status.toString().replaceAll("_", " "); to get it's string version.

I don't know your skill level so this might be a bit advanced for you, but if you are willing to learn, I really suggest that you start playing around with enums. You will love them :)

Spacing

`}else if(attending.isChecked()){`

is a bit tightly written, you can increase readability a bit by using more spaces, like this:

`} else if (attending.isChecked()) {`

Private, final, and constants

There are plenty of instance variables in your code that could be private, final, and possibly static.

String ATTENDING = "ATTENDING";
String NOT_ATTENDING = "NOT ATTENDING";
String INVITED = "INVITED";

All these are constants, they should never change, and should therefore be declared private static final

private static final String ATTENDING = "ATTENDING";
private static final String NOT_ATTENDING = "NOT ATTENDING";
private static final String INVITED = "INVITED";

I believe that all of your variable declarations above your constructor can be marked private, and I also believe that all them can be final:

private final Context context; 
private final int layoutResourceId;    
private final ArrayList<People_Attendance> people_attendance = null;
private final String[] attendance_status;

Making them private will let the compiler tell you if any of them are not used, and making them final will ensure you that once initialized in the constructor, their value / object-reference cannot change.

Naming

Using class names with _ in Java is unusual and not convention, PeopleAttendance would be a better name. Also, class names should start with an upper-case letter and use CamelCasing, making attendance_holder --> AttendanceHolder.

Overall though, I get the impression that you seem to know what you are doing coding-wise, good work :)

\$\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.