2

is there a way to make the following script more efficient? I want to make this code to be easy maintainable, thats why I'd like to get rid of the if else if's. I hope you guys can help me out here. In the bottom is something I would like to see, if its possible that way of course.

     if (category.equals("infusion")){
        layout.setBackgroundResource(R.color.infusion);
            title.setText(R.string.title_infusion);             
     } else if (category.equals("pills")){
            layout.setBackgroundResource(R.color.pills);
            title.setText(R.string.title_pills);
     } else if (category.equals("pumps")){
            layout.setBackgroundResource(R.color.pumps);
            title.setText(R.string.title_pumps);
     } else if (category.equals("oxygen")){
            layout.setBackgroundResource(R.color.oxygen);
            title.setText(R.string.title_oxygen);
     } else if (category.equals("scores")){
            layout.setBackgroundResource(R.color.scores);
            title.setText(R.string.title_scores);
     } else if (category.equals("converters")){
            layout.setBackgroundResource(R.color.converters);
            title.setText(R.string.title_converters);
     }

Something like this?

layout.setBackgroundResource(R.color.*category*);
title.setText(R.string.title_*category*);

6 Answers 6

3

I'm pretty sure that all of the things you would do the "simplify" this would involve reflection, and would probably ending up making your code slower and harder to understand. What you have is a perfectly valid way of doing this set up, is very clear to the reader and doesn't involve any weird techniques.

I.E. This works, why fix it?

Ok, Edit:

At start up you could map your String values to resource ids via hash map.

Something like:

HashMap map = new HashMap();
map.put("infusion",R.id.infusion);

and then later on:

layout.setBackgroundResource(map.get(category));
title.setText(category);   

That might work, but again it's not really an improvement imo.

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

Comments

2

Check out Resources.getIndentifier() could have a helper function like:

public static int resourceNameToId(Context context, String name, String resType) {
    if (name != null && name.length() > 0) {
        return context.getResources().getIdentifier(name, resType, context.getPackageName());
    }

    return 0;
}

Then to use it:

layout.setBackgroundResource(resourceNameToId(getContext(), category, "color"));

4 Comments

I should also say that this uses reflection internally and should be used lightly
From the docs: Note: use of this function is discouraged. It is much more efficient to retrieve resources by identifier than by name.
Yup, I wouldn't suggest putting in an adapter, but it can be pretty useful for dynamically generated content. Also by efficient, I think OP is more interested in maintainability
I'm not actually sure what the OP means by efficient at all, I think he wants non conditional code. That said the enum approach mentioned is a good one.
2

You could use an enum like this that hold your values and colors:

public enum ColorValue {

InFusion(android.R.color.black),
Pills(android.R.color.white);

int color;

ColorValue(int Value) {
    color = Value;
}

public int getColorResource() {
    return color;
}

} 

Then access the enum value similar to this

 ColorValue x=ColorValue.InFusion;
 x.getColorResource();

Comments

0

You can't achieve this with your code construction.

Use something like:

layout.setBackgroundResource(getColorByCategory(category));
title.setText(getCategoryTitle(category));

where getColorByCategory() and getCategoryTitle() is your own functions.

Comments

0

There's 2 ways I can think off the top of my head. A hash map and or using the Resources object.

A has map would involve a little setup, like so:

private static final Map<String, Integer> COLOURS = new HashMap<String, Integer>();

static {
    COLOURS.put("pills", R.color.red);
    COLOURS.put("pumps", R.color.green);
}

Then it's very easy to set the colours later on:

layout.setBackgroundColor(COLOURS.get(category));

You might want to null check the colours coming out of the map before setting them if the data you have isn't guaranteed to be perfect.

Alternatively, you can use the Resources object, like this.

Resources resources = context.getResources();
resources.getColor(resources.get("colour_" + category, "color", context.getPackageName()));

Comments

0

You could use an enum to do this more effeciently:

(In your class) declare

public enum Category {
    infusion (R.color.infusion, R.string.title_infusion, "infusion"),
    pills (R.color.pills, R.string.title_pills, "pills") //,
    //etc... comma-seperated values. Those are handled like any other enum
    ;
    public final int relatedBackground;
    public final String relatedTitle;
    public final String identifier; //To keep the identifiers you used previously

    private Category (int back, String title, String id) {
        this.relatedBackground = back;
        this.relatedTitle = title;
        this.identifier = id;
    }
}

And your method would look like this:

public void foo(Category cat) { //You pass in your enum type
    //Do what you want
    if(cat != null) { //Avoid NullPointerException if necessary
        layout.setBackgroundResource(cat.relatedBackground); 
        title.setText(cat.relatedTitle);
    }
}

This method is a great way to solve the problem you got because you can maintain you project by adding a new value to the comma-seperated list. You also don't need to worry about having to look up the values everytime from some table.

A downside is that it is hard/impossible to tweak the enum during runtime. As all variables in an enum have to be final as the enumtype itself is not modifiable you can't do things like

cat.relatedbackground = someValue; // Can't do that: all fields are final

If you want to do that you would have to use an extern table. Apart from that it is an excellent solution.

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.