8
\$\begingroup\$

I thought I would learn a design pattern today and picked this one. I wrote a simple Android test demo to test the pattern.

main.xml

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
              android:orientation="vertical"
              android:layout_width="fill_parent"
              android:layout_height="fill_parent"
        >

    <Spinner
        android:id="@+id/select"
        android:layout_height="wrap_content"
        android:layout_width="fill_parent" />

    <EditText
            android:id="@+id/editOne"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:text="First"
            android:editable="false"/>

    <EditText
            android:id="@+id/editTwo"
            android:layout_width="fill_parent"
            android:layout_height="wrap_content"
            android:text="Second"
            android:editable="false"/>

</LinearLayout>

strings.xml

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <string name="app_name">TestingStrategyPattern</string>
    <string-array name="names">
        <item>Queued</item>
        <item>In Progress</item>
        <item>Started</item>
        <item>Finished</item>
        <item>Destroyed</item>
        <item>Bombarded</item>
        <item>Ready</item>
        <item>Paused</item>
        <item>Stopped</item>
        <item>Resolved</item>
        <item>Abandoned</item>
    </string-array>
</resources>

MyActivity.java

package com.example.TestingStrategyPattern;

import android.app.Activity;
import android.os.Bundle;
import android.view.View;
import android.widget.AdapterView;
import android.widget.ArrayAdapter;
import android.widget.EditText;
import android.widget.Spinner;

public class MyActivity extends Activity {

    private boolean editOneEnabled = false;
    private boolean editTwoEnabled = false;
    /**
     * Called when the activity is first created.
     */
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);

        Spinner statesSpinner = (Spinner) findViewById(R.id.select);
        ArrayAdapter<CharSequence> adapter = ArrayAdapter.createFromResource(this, R.array.names, android.R.layout.simple_spinner_dropdown_item);
        adapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
        statesSpinner.setAdapter(adapter);

        final EditText editOne = (EditText) findViewById(R.id.editOne);
        final EditText editTwo = (EditText) findViewById(R.id.editTwo);

        statesSpinner.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() {
            @Override
            public void onItemSelected(AdapterView<?> parent, View view, int position, long id) {
                String selectedItem = parent.getSelectedItem().toString();

                if(selectedItem.equals("Queued")) {
                    editOneEnabled = true;
                    editTwoEnabled = false;
                } else if(selectedItem.equals("In Progress")){
                    editOneEnabled = true;
                    editTwoEnabled = false;
                } else if(selectedItem.equals("Started")) {
                    editOneEnabled = true;
                    editTwoEnabled = false;
                } else if(selectedItem.equals("Bombarded")) {
                    editOneEnabled = true;
                    editTwoEnabled = false;
                } else if(selectedItem.equals("Ready")) {
                    editOneEnabled = false;
                    editTwoEnabled = true;
                } else if(selectedItem.equals("Paused")) {
                    editOneEnabled = false;
                    editTwoEnabled = true;
                } else if(selectedItem.equals("Stopped")) {
                    editOneEnabled = false;
                    editTwoEnabled = true;
                } else if(selectedItem.equals("Resolved")) {
                    editOneEnabled = false;
                    editTwoEnabled = true;
                } else if(selectedItem.equals("Abandoned")) {
                    editOneEnabled = true;
                    editTwoEnabled = false;
                } else {
                    editOneEnabled = false;
                    editTwoEnabled = false;
                }

                if(editOneEnabled == true) {
                    editOne.setEnabled(true);
                } else {
                    editOne.setEnabled(false);
                }

                if(editTwoEnabled == true) {
                    editTwo.setEnabled(true);
                } else {
                    editTwo.setEnabled(false);
                }
            }

            @Override
            public void onNothingSelected(AdapterView<?> parent) {

            }
        });

    }
}

Now this is the code before I refactored it to use the strategy pattern, but effectively what happens is that based on the selection from the drop down/spinner widget, the two text fields will get enabled or disabled accordingly.

Here is the refactored code:

Strategy.java

package com.example.strategy;

public interface Strategy {
    boolean isEnabled(String state);
}

Context.java

package com.example.strategy;

public class Context {
    private Strategy strategy;

    public Context(Strategy strategy) {
        this.strategy = strategy;
    }

    public boolean executeStrategy(String state) {
        return this.strategy.isEnabled(state);
    }
}

ButtonOneStrategy.java

package com.example.strategy;

import java.util.HashMap;
import java.util.Map;

public class ButtonOneStrategy implements Strategy {

    private Map<String, Boolean> mappings;

    public ButtonOneStrategy() {
        mappings = new HashMap<String, Boolean>();
        mappings.put("Queued", true);
        mappings.put("In Progress", true);
        mappings.put("Started", true);
        mappings.put("Bombarded", true);
        mappings.put("Ready", false);
        mappings.put("Paused", false);
        mappings.put("Stopped", false);
        mappings.put("Resolved", false);
        mappings.put("Abandoned", false);
        mappings.put("Destroyed", false);
        mappings.put("Finished", false);
    }

    @Override
    public boolean isEnabled(String state) {
        return mappings.get(state);
    }
}

ButtonTwoStrategy.java

package com.example.strategy;

import java.util.HashMap;
import java.util.Map;

public class ButtonTwoStrategy implements Strategy {

    private Map<String, Boolean> mappings;

    public ButtonTwoStrategy() {
        mappings = new HashMap<String, Boolean>();
        mappings.put("Queued", false);
        mappings.put("In Progress", false);
        mappings.put("Started", false);
        mappings.put("Bombarded", false);
        mappings.put("Ready", true);
        mappings.put("Paused", true);
        mappings.put("Stopped", true);
        mappings.put("Resolved", true);
        mappings.put("Abandoned", true);
        mappings.put("Destroyed", false);
        mappings.put("Finished", false);
    }

    @Override
    public boolean isEnabled(String state) {
        return mappings.get(state);
    }
}

MyActivity.java

package com.example.TestingStrategyPattern;

import android.app.Activity;
import android.os.Bundle;
import android.view.View;
import android.widget.AdapterView;
import android.widget.ArrayAdapter;
import android.widget.EditText;
import android.widget.Spinner;
import com.example.strategy.ButtonOneStrategy;
import com.example.strategy.ButtonTwoStrategy;
import com.example.strategy.Context;

public class MyActivity extends Activity {

    private boolean editOneEnabled = false;
    private boolean editTwoEnabled = false;
    /**
     * Called when the activity is first created.
     */
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);

        Spinner statesSpinner = (Spinner) findViewById(R.id.select);
        ArrayAdapter<CharSequence> adapter = ArrayAdapter.createFromResource(this, R.array.names, android.R.layout.simple_spinner_dropdown_item);
        adapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
        statesSpinner.setAdapter(adapter);

        final EditText editOne = (EditText) findViewById(R.id.editOne);
        final EditText editTwo = (EditText) findViewById(R.id.editTwo);
        final com.example.strategy.Context btnOneContext = new Context(new ButtonOneStrategy());
        final com.example.strategy.Context btnTwoContext = new Context(new ButtonTwoStrategy());

        statesSpinner.setOnItemSelectedListener(new AdapterView.OnItemSelectedListener() {
            @Override
            public void onItemSelected(AdapterView<?> parent, View view, int position, long id) {
                String selectedItem = parent.getSelectedItem().toString();

                editOneEnabled = btnOneContext.executeStrategy(selectedItem);
                editTwoEnabled = btnTwoContext.executeStrategy(selectedItem);

                if(editOneEnabled == true) {
                    editOne.setEnabled(true);
                } else {
                    editOne.setEnabled(false);
                }

                if(editTwoEnabled == true) {
                    editTwo.setEnabled(true);
                } else {
                    editTwo.setEnabled(false);
                }
            }

            @Override
            public void onNothingSelected(AdapterView<?> parent) {

            }
        });

    }
}

I added Strategy.java, Context.java, ButtonOneEnabled.java, ButtonTwoEnabled.java and changed MyActivity.java as a result of the new pattern.

Here are my questions:

  1. What do you think about how I went about implementing this pattern? Have I done it in a good way?
  2. I felt that this pattern was overkill for the example. Could someone explain the benefits of using this pattern in larger examples or indeed in this example? I really felt that although this pattern could scale really well, it felt a bit of an over design, creating more objects with their own maps etc. I thought it was too heavy. What do you think about this?
\$\endgroup\$
2
  • 3
    \$\begingroup\$ Design patterns are by their definition best practice solutions to (programming) problems. The problem in your original code you're obviously trying to solve is the scary if-elseif-chain. The Strategy pattern comes into play when you want to make a part of an algorithm exchangable and decouple this part from its client. IMHO, the pattern does not really fit your problem. You should have a look at the Command pattern. \$\endgroup\$
    – conrad
    Commented Dec 18, 2014 at 0:27
  • \$\begingroup\$ @conrad thank you, I will take a look and implement something :) \$\endgroup\$ Commented Dec 18, 2014 at 22:26

2 Answers 2

4
\$\begingroup\$

The Strategy pattern is more useful when the implementations are significantly different, and typically done using multiple classes implementing the same interface. In your case, the implementation is possible using a single class, for example:

class ValueMatchingStrategy implements Strategy {

    private final Set<String> values;

    public ValueMatchingStrategy(String... values) {
        this.values = new HashSet<String>(Arrays.asList(values));
    }

    @Override
    public boolean isEnabled(String value) {
        return values.contains(value);
    }
}

And then in your code you could create two instances of this class, for example:

Strategy buttonOneStrategy = new ValueMatchingStrategy("Queued", "In Progress", "Started", "Bombarded");
Strategy buttonTwoStrategy = new ValueMatchingStrategy("Ready", "Paused", "Stopped", "Resolved", "Abandoned");

This part can be vastly simplified:

            if(editOneEnabled == true) {
                editOne.setEnabled(true);
            } else {
                editOne.setEnabled(false);
            }

            if(editTwoEnabled == true) {
                editTwo.setEnabled(true);
            } else {
                editTwo.setEnabled(false);
            }

To simply this:

            editOne.setEnabled(editOneEnabled);
            editTwo.setEnabled(editTwoEnabled);

Finally, but very importantly, all the names you used in the strategy pattern logic are strange, unnatural. editOneEnabled and editTwoEnabled clearly sound like hypothetical code and made it a bit difficult to review your code. executeStrategy is really a meaningless name. A method name with "execute" in it sounds more like the Command pattern. The Strategy interface shouldn't really be called "Strategy", and its method shouldn't really be called "executeStrategy", but be more meaningful for the purpose that it's supposed to accomplish.

In any case, and as I explained above, since a single implementation can handle both of your use cases, the Strategy pattern doesn't seem necessary here. We don't know how you expect your code to evolve. Whether it's worth the investment depends on your use case, requirements, and likely changes you anticipate.

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

A few more notes besides what @janos wrote:

android:text="Second"

Don't hardcode strings in your layout.xml. Use a String resource.

android:text="@strings/second_button"

com.example.strategy.Context

That is a really bad name for that class. Here you have to write the fully qualified class name because of Android's Context class, which is not something you'd want often.

Moreover, I don't really see the purpose of that class at all. It is just a wrapper for a Strategy, and an unnecessary one. This part can be changed:

com.example.strategy.Context btnOneContext = new Context(new ButtonOneStrategy());
editOneEnabled = btnOneContext.executeStrategy(selectedItem);

And written as:

final Strategy btnOneContext = new ButtonOneStrategy();
editOneEnabled = btnOneContext.isEnabled(selectedItem);

Violà, one unnecessary class removed!

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.