principlejavaMinor
Does this code fall into a design pattern?
Viewed 0 times
thisintodesigndoescodefallpattern
Problem
CustomDialog is the parent abstract class for OkExclamationDialog and ExclamationDialog with abstract method displayDialog() being overriden.Does this fall into a design pattern? Can this design be improved upon?
JudgeButton controls the other classes.```
public class JudgeButton {
private int typeCode;
private void initialise(){
setTypeCode();
if(typeCode == 0){
c = new ExclamationDialog("Episode not started");
c.displayDialog();
}
else if(typeCode == 1){
c = new OkExclamationDialog("Episode Finished, you will now be redirected");
c.displayDialog();
}
}
private void setTypeCode(){
wrapperJsonObject = Utils.getCurrentWrapperJsonObjectFromServer();
jsonObject = wrapperJsonObject.getJsonObject();
NextRatingsScreen n = new NextRatingsScreen(jsonObject);
RatingsCountDownTime r = new RatingsCountDownTime();
if(!n.isEpisodeStarted()){
typeCode = 0;
}
else if(!n.isPerformanceStarted()){
typeCode = 1;
}
}
}
public class OkExclamationDialog extends CustomDialog{
public OkExclamationDialog(String text) {
super(text);
}
public void displayDialog() {
DialogType dialogType = new DialogType();
DialogBuilder dialogBuilder = new DialogBuilder();
AbstractDialogFactory abstractDialogFactory = dialogType.getDialogType(DialogEnum.EXCLAMATION_DIALOG);
Dialog nextScreen = dialogBuilder.buildDialog(abstractDialogFactory, super.getText());
int result = nextScreen.doModal();
ScreenController.displayNextScreenFadeTransition(nextScreen);
if(result==Dialog.OK)
{
ScreenController.displayNextScreenFadeTransition(new ContestantsScreen());
}
}
}
public class ExclamationDialog extends CustomDialog{
public ExclamationDialog(String text) {
super(text);
}
publ
Solution
The
Just some idea:
1, Consider using an
2, Maybe you should throw an
3, The same is true for the
4, I'd rename the
5, You should create a new
initialise reminds me a factory but it's not exactly a factory. It's fine if you don't use the same if-elseif structure in your code elsewhere. If you do pull out it to a factory class.Just some idea:
1, Consider using an
enum instead of the integer typeCode (eliminate magic numbers).2, Maybe you should throw an
IllegalStateException in the end of the initialise method with a message "Invalid typecode".3, The same is true for the
setTypeCode() method. Or set explicitly typeCode to 0 in the last line of the method if 0 is a valid value. (This is the implicit default value of the private int typeCode field.)4, I'd rename the
setTypeCode() to calculateTypeCode() which would return a TypeCode enum. Now it's more or less temporal coupling.5, You should create a new
getNextScreen() method in the CustomDialog class with the first four line of the displayDialog() method. Then call the getNextScreen() from the displayDialog() methods. It would remove some code duplication.// CustomDialog class code
public Dialog createNextScreen() {
DialogType dialogType = new DialogType();
DialogBuilder dialogBuilder = new DialogBuilder();
AbstractDialogFactory abstractDialogFactory =
dialogType.getDialogType(DialogEnum.EXCLAMATION_DIALOG);
Dialog nextScreen = dialogBuilder.buildDialog(abstractDialogFactory, text);
return nextScreen;
}// OkExclamationDialog, ExclamationDialog class code
public void displayDialog() {
Dialog nextScreen = getNextScreen();
...
ScreenController.displayNextScreenFadeTransition(nextScreen);
}Code Snippets
// CustomDialog class code
public Dialog createNextScreen() {
DialogType dialogType = new DialogType();
DialogBuilder dialogBuilder = new DialogBuilder();
AbstractDialogFactory abstractDialogFactory =
dialogType.getDialogType(DialogEnum.EXCLAMATION_DIALOG);
Dialog nextScreen = dialogBuilder.buildDialog(abstractDialogFactory, text);
return nextScreen;
}// OkExclamationDialog, ExclamationDialog class code
public void displayDialog() {
Dialog nextScreen = getNextScreen();
...
ScreenController.displayNextScreenFadeTransition(nextScreen);
}Context
StackExchange Code Review Q#5775, answer score: 3
Revisions (0)
No revisions yet.