2

I have RecyclerView adapter class which is showing information about users from (User class which takes Data from the Server) my senior told me that: something wrong with your code, you should figure out it, and he said that I should use String.format() and also he said: (if else is not a good solution, What if there were 100 users instead of 10? Or if new users were added every day, you should optimize your code and lastly, he said Look for String.format and %d in it).

The App works normal but I didn't understand what he wants exactly to do, I tried to find a solution on the internet but I didn't. I need your help, please.

My Adapter class:

public class UserAdapter extends RecyclerView.Adapter<UserAdapter.UserViewHolder> {
    private List<User> userList;
    private Context context;
    private OnItemClickListener listener;

    public UserAdapter(List<User> userList, Context context) {
        this.userList = userList;
        this.context = context;
    }

    @NonNull
    @Override
    public UserViewHolder onCreateViewHolder(@NonNull ViewGroup parent, int viewType) {
        View view = LayoutInflater.from(parent.getContext()).inflate(R.layout.users_layout, parent, false);
        return new UserViewHolder(view);
    }

    @Override
    public void onBindViewHolder(@NonNull UserViewHolder holder, int position) {
        User user = userList.get(position);
        if (user != null) {
            holder.imageView.setAnimation(AnimationUtils.loadAnimation
                    (context, R.anim.fade_transition_anim));
            holder.linearLayout.setAnimation(AnimationUtils.loadAnimation
                    (context, R.anim.fade_scale_animation));
            holder.txtName.setText(user.getName());
            holder.txtEmail.setText(user.getEmail());
            holder.txtInfo.setText(user.getCompany().getCatchPhrase());

            switch (user.getId()) {
                case 1:
                    Glide.with(context).load("https://avatars.io/twitter/1").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 2:
                    Glide.with(context).load("https://avatars.io/twitter/2").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 3:
                    Glide.with(context).load("https://avatars.io/twitter/3").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 4:
                    Glide.with(context).load("https://avatars.io/twitter/4").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 5:
                    Glide.with(context).load("https://avatars.io/twitter/5").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 6:
                    Glide.with(context).load("https://avatars.io/twitter/6").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 7:
                    Glide.with(context).load("https://avatars.io/twitter/7").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 8:
                    Glide.with(context).load("https://avatars.io/twitter/8").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 9:
                    Glide.with(context).load("https://avatars.io/twitter/9").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                case 10:
                    Glide.with(context).load("https://avatars.io/twitter/10").placeholder(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
                default:
                    Glide.with(context).load(R.drawable.default_128)
                            .into(holder.imageView);
                    break;
            }
        }
    }

    @Override
    public int getItemCount() {
        int a;
        if (userList != null && !userList.isEmpty()) {
            a = userList.size();
        } else {
            a = 0;
        }
        return a;
    }

    public class UserViewHolder extends RecyclerView.ViewHolder {
        private CircleImageView imageView;
        private TextView txtName, txtEmail, txtInfo;
        private LinearLayout linearLayout;

        public UserViewHolder(@NonNull View itemView) {
            super(itemView);
            imageView = itemView.findViewById(R.id.img_user);
            txtName = itemView.findViewById(R.id.txt_user_name);
            txtEmail = itemView.findViewById(R.id.txt_user_email);
            txtInfo = itemView.findViewById(R.id.txt_user_info);
            linearLayout = itemView.findViewById(R.id.anim_container);

            itemView.setOnClickListener(new View.OnClickListener() {
                @Override
                public void onClick(View v) {
                    if (listener != null) {
                        int position = getAdapterPosition();
                        if (position != RecyclerView.NO_POSITION) {
                            listener.OnItemClick(position);
                        }
                    }
                }
            });
        }
    }

    public interface OnItemClickListener {
        void OnItemClick(int position);
    }

    public void setOnItemClickListener(OnItemClickListener listener) {
        this.listener = listener;
    }
}
12
  • @Tim Ok now i got it,thank you so much, i asked him what does he mean but he said you should figure it out by yourself,Thank you Again Commented Nov 20, 2019 at 14:03
  • Also note: there are plenty of other places in your code that could be improved. You should seriously ask your "senior" to sit down with you and do a line by line review of your code. When a person learning Java, and Android and whatnot at the same time ... then it is asked too much that the newbie also writes high quality code at the same time. You see, your code works. The point is that there are just many places that could be improved. For exampole your getItemCount() method is overly complicated. You could just do: return (userList == null) ? 0 : userList.size() instead! Commented Nov 20, 2019 at 14:26
  • @GhostCatsaysReinstateMonica ok, Thank you again, i will do as you said,you helped me a lot:) are there any other places in my code except getItemCount() ? :))) Commented Nov 20, 2019 at 14:30
  • This place isnt meant for extensive code reviews. As said: it is best that you find real humans willing to sit down with you. Alternatively, see codereview.stackexchange.com/help/on-topic ... and then write up a question that fits that community, to ask for feedback on specific aspects of your code. Commented Nov 20, 2019 at 14:42
  • @GhostCatsaysReinstateMonica OK,i understood,i really don't know how to thank you.You are really amazing person Commented Nov 20, 2019 at 14:47

2 Answers 2

2

What your senior tells you is:

case 1:
    Glide.with(context).load("https://avatars.io/twitter/1")

that is madness.

Meaning: all your switch cases are the same. The only differ in that number.

And such repeating details, like a number that needs to go into a string, can, for example be solved using String.format(), like:

Glide.with(context).load(String.format("https://avatars.io/twitter/%i", theNumber), ...

So, the real answer here is: always pay attention to the code you are writing. Latest when your code is functional complete, and you think "yes, it is working", then you should carefully look at your code, and for example: identify code duplication, to then find ways to avoid that.

Sign up to request clarification or add additional context in comments.

Comments

0

you are implemented code with static value like..
Glide.with(context).load("https://avatars.io/twitter/1")
Your code have to support dynamic value without using switch case

1) he said Look for String.format and %d in it).
--> Glide.with(context).load(String.format("https://avatars.io/twitter/%d", user.getId()), ...

for more detail for String.format, refer java document ==> https://docs.oracle.com/javase/7/docs/api/java/util/Formatter.html

2) Also use String.valueOf() method.

Glide.with(context).load("https://avatars.io/twitter/"+String.valueOf(user.getId())).placeholder(R.drawable.default_128).into(holder.imageView);

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.