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

Does this code fall into a design pattern?

Submitted by: @import:stackexchange-codereview··
0
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 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.