principlejavaMinor
Strategy Pattern Implementation
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
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"
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:
And then in your code you could create two instances of this class, for example:
This part can be vastly simplified:
To simply this:
Finally, but very importantly, all the names you used in the strategy pattern logic are strange, unnatural.
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.
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.