HiveBrain v1.2.0
Get Started
← Back to all entries
principlejavaMinor

Strategy Pattern Implementation

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationpatternstrategy

Problem

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


    

    

    


strings.xml


    TestingStrategyPattern
    
        Queued
        In Progress
        Started
        Finished
        Destroyed
        Bombarded
        Ready
        Paused
        Stopped
        Resolved
        Abandoned
    


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 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"

Solution

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 values;

    public ValueMatchingStrategy(String... values) {
        this.values = new HashSet(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.

Code Snippets

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);
    }
}
Strategy buttonOneStrategy = new ValueMatchingStrategy("Queued", "In Progress", "Started", "Bombarded");
Strategy buttonTwoStrategy = new ValueMatchingStrategy("Ready", "Paused", "Stopped", "Resolved", "Abandoned");
if(editOneEnabled == true) {
                editOne.setEnabled(true);
            } else {
                editOne.setEnabled(false);
            }

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

Context

StackExchange Code Review Q#74006, answer score: 4

Revisions (0)

No revisions yet.