2

alright, I voted to delete my previous similar question to keep it clean. The matter: I'm trying to use a custom ListView for displaying messages in a chat app. I now managed to set the text according to user's input but I have some troubles in the getView() method. You can see the problem at the pictures below: the inflated layouts are added at the wrong position.

enter image description here

This should start with test and end with test5. I suppose the getView() method should have been called 6 times since 6 rows were inflated? But I added a Log to this method and figured out that it was called like 20+ times! Why so?

enter image description here

My code goes below:

the adapter class:

public final class ChatAdapter extends BaseAdapter {
private Context context;
ViewHolder holder;

private ArrayList<String> messages;

public ChatAdapter(Context context, ArrayList<String> chat) {

    this.context = context;
    this.messages = chat;
    Log.d("ChatAdapter", "called constructor");

}

public int getCount() {
    return messages.size();
}

public Object getItem(int position) {
    return messages.get(position);
}

public long getItemId(int position) {
    return position;
}

public View getView(int position, View convertView, ViewGroup viewGroup) {

    if (convertView == null) {
        LayoutInflater inflater = (LayoutInflater) context
                .getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        convertView = inflater.inflate(R.layout.row, null);

        holder = new ViewHolder();
        holder.message = (TextView) convertView
                .findViewById(R.id.tvMessage);
        holder.timestamp = (TextView) convertView.findViewById(R.id.tvTS);
        holder.indicator = (ImageView) convertView
                .findViewById(R.id.imgInd);

        String message = (String) getItem(position);
        holder.message.setText(message);
        holder.timestamp.setText(generateTimestamp());

        convertView.setTag(holder);

    } else {
        holder = (ViewHolder) convertView.getTag();

    }
    Log.d("ChatAdapter", "called getView");
    return convertView;
}

@SuppressLint("SimpleDateFormat")
public String generateTimestamp() {
    SimpleDateFormat sdf = new SimpleDateFormat("HH:mm");
    String ts = sdf.format(new Date());

    return ts;
}

public static class ViewHolder {
    public TextView message;
    public TextView timestamp;
    public ImageView indicator;
}

The ChatActivity:

public class ChatActivity extends ListActivity {

private EditText input;
private String tmpMessage;
ListView lv;
ChatAdapter adapter;
ArrayList<String> messages;

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.lis);
    Button send = (Button) findViewById(R.id.btnSend);
    input = (EditText) findViewById(R.id.etInput);
    lv = getListView();

    messages = new ArrayList<String>();
    Log.d("Chat", "arrayList created");
    adapter = new ChatAdapter(this, messages);
    lv.setOnItemClickListener(new OnItemClickListener() {

        @Override
        public void onItemClick(AdapterView<?> arg0, View arg1, int arg2,
                long arg3) {
            // TODO
        }
    });

    lv.setAdapter(adapter);

    send.setOnClickListener(new View.OnClickListener() {

        @Override
        public void onClick(View v) {
            sendMessage();
        }
    });
}

public void sendMessage() {

    tmpMessage = input.getText().toString();
    input.setText("");


    messages.add(tmpMessage);

    adapter.notifyDataSetChanged();

}

What am I missing (except of some brain)? Any help is greatly appreciated.

Droidman
  • 11,485
  • 17
  • 93
  • 141

1 Answers1

1

I suppose the getView() method should have been called 6 times since 6 rows were inflated? But I added a Log to this method and figured out that it was called like 20+ times! Why so?

A common cause that forces getView() to be called multiple times is using wrap_content for the ListView's height.

Also this code in getView():

String message = (String) getItem(position);
holder.message.setText(message);
holder.timestamp.setText(generateTimestamp()); // You will need to make a permanent timestamp someday

Should be outside the if-else block to support scrolling.

Sam
  • 86,580
  • 20
  • 181
  • 179
  • thanks a lot, this solved the problem. BTW, I changed the dimensions of the ListView to match_parent, but getView() keeps getting called multiple times.. But the messages are now added correctly after I've repositioned the code you mentioned. Can any performance issues occur cause of that? – Droidman Jan 27 '13 at 13:38
  • I'm not entirely sure why `getView()` is called so many times, I see you are using an EditText, does the keyboard open right away when you start the Activity? (The keyboard opening / closing will cause the layout to be redrawn.) As far as performance issues: if you don't notice a delay, you can often ignore it. The general rule is don't spend hours / days optimizing your code if you are only going to save milliseconds. – Sam Jan 28 '13 at 19:34
  • the keyboard does not get opened immediately since I've set the android:windowSoftInputMode="stateAlwaysHidden" property. Didn't know the layout is redrawn each time the keyboard appears.. But still there are too many calls on the getView() method, can't figure out the reason – Droidman Jan 29 '13 at 08:50
  • by the way, do I need to change the constructor and use an extra ArrayList for the timestamp? Don't see any other possibility to prevent it from being changed after getView() is called again with new time, old timestamp just gets overwritten for all messages – Droidman Jan 29 '13 at 11:20
  • "do I need to change the constructor and use an extra ArrayList for the timestamp?" You could keep two separate lists, but this is prone to errors. (At some point in the future, you will inevitably add or remove data from one list but not the other...) It might be best to create a simple ChatText class (or a table in a database) that keeps each String, timestamp, and any other data together into one object. – Sam Jan 29 '13 at 17:36