How can i trim my code?

Question

In my code, I have many codeblocks of these... for example:

if(Avatar==1) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar1);
if(Avatar==2) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar2);
if(Avatar==3) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar3);
if(Avatar==4) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar4);
if(Avatar==5) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar5);
if(Avatar==6) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar6);
if(Avatar==7) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar7);
if(Avatar==8) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar8);
if(Avatar==9) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar9);
if(Avatar==10) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar10);
if(Avatar==11) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar11);
if(Avatar==12) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar12);
if(Avatar==13) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar13);
if(Avatar==14) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar14);
if(Avatar==15) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar15);
if(Avatar==16) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar16);
if(Avatar==17) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar17);
if(Avatar==18) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar18);
if(Avatar==19) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar19);
if(Avatar==20) ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.avatar20);

For example in PHP i can write:

**$STRINGavatar** = "avatar20";
echo"((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(R.drawable.**$STRINGavatar**)";

But in Java it doesn't works :-(

@Jason how are enums going to shrink that code? – tyczj Jul 12 '17 at 13:35 — tyczj, Jul 12 '17 at 13:35

andrei · Accepted Answer · 2017-07-12T13:48:10.647

3

Your could do something like

getContext().getResources().getIdentifier("avatar" + i, "drawable", getContext().getPackageName())

to get the resource based on the name.

[EDIT]

And then your code would be :

int resId = getContext().getResources().getIdentifier("avatar" + Avatar, "drawable", getContext().getPackageName());
((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(resId);

And your variable Avatar should start with a lower case

edited Jul 12 '17 at 13:48

answered Jul 12 '17 at 13:39

andrei

2,934
2
23
36

This is definitely better than my answer – MacLean Sochor Jul 12 '17 at 13:41
1

Isn't this unsafe and non-refactorable? – Joey Pinto Jul 12 '17 at 13:49
@JoeyPinto It will also fail at runtime if the filenames are incorrect, whereas something like my answer will fail at compile time. – Michael Jul 12 '17 at 13:54
@JoeyPinto if by unsafe you mean what happens if the resource is not found, getIndentifier returns 0 and the image is not displayed. So the app will not crash, but there will be an image missing from the UI. And what do you mean by refactorable ? – andrei Jul 12 '17 at 13:55
What if filenames have to be changed? You are hard coding things, not a characteristic of good code. Reflection also brings in security concerns. – Joey Pinto Jul 12 '17 at 13:56
@JoeyPinto you are talking about filenames, do you mean if we want to change the prefix, "avatar" ? You are right, we could get this from a config file, a constant, ws response, or any other place, but I don't have a global view of the project. – andrei Jul 12 '17 at 14:05
Check out my answer below – Joey Pinto Jul 12 '17 at 14:35

score 1 · Answer 2 · answered Jul 12 '17 at 13:41

Well for starters, use a function for the part you've obviously copied and pasted.

if (Avatar==1) setImage(R.drawable.avatar1);
if (Avatar==2) setImage(R.drawable.avatar2);
if (Avatar==3) setImage(R.drawable.avatar3);
//...    

private void setImage(final int resource)
{
    ((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(resource);
}

That at least shrinks it horizontally and removes a lot of the duplication.

The next thing would be to move the mapping between integers and resources to a single place, especially if you need to query this more than once:

private int getResource(final int avatar)
{
    switch(avatar)
    {
        case 1: return R.drawable.avatar1;
        case 2: return R.drawable.avatar2;
        case 3: return R.drawable.avatar3;
        //...
        default:
            throw new RuntimeException("No avatar for this");
    }
}

And then you may change the above code to:

setImage(getResource(avatar));

score 0 · Answer 3 · answered Jul 12 '17 at 13:39

0

You should try to do it in different lines

ImageView iv = (ImageView) dialogPopup.findViewById(R.id.imgView);
iv.setImageResource(R.drawable.avatar20)

answered Jul 12 '17 at 13:39

solamente

266
2
15

score 0 · Answer 4 · answered Jul 12 '17 at 13:39

0

You can use getIdentifier for the drawable

int id = getResources().getIdentifier(imageName, "drawable", getPackageName());

answered Jul 12 '17 at 13:39

tyczj

71,600
54
194
296

score 0 · Answer 5 · answered Jul 12 '17 at 13:41

0

Get the resource id dynamically:-

int id = getResources().getIdentifier("avatar" + Avatar, "drawable", getPackageName());
((ImageView)(dialogPopup.findViewById(R.id.imgView))).setImageResource(id);

answered Jul 12 '17 at 13:41

Steve Smith

2,244
2
18
22

score 0 · Answer 6 · answered Jul 12 '17 at 13:47

0

Try this, use an enum structure as follows

public enum Avatar {
   R.drawable.avatar1, R.drawable.avatar2, ..R.drawable.avatar20;
   public static final Avatar values[] = values();
}
 public static void main(String []args){
   Drawable avatar = Avatar.values[1];//just put in the number to get the drawable
 }
}

answered Jul 12 '17 at 13:47

Joey Pinto

1,735
1
18
34

This will not even build. It's not an enum you have there. And if you actually put all the drawables in an enum, depending on the images, I think you can get an OOM – andrei Jul 12 '17 at 14:41

How can i trim my code?

6 Answers6