Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am developing game portal content management system using java and j2ee tehcnology. However i am stuck in my design implementation.

Here is the scenario:

Each Game object has attributes like id, name, developer, price, platformType, platformName and so on...

However the release date may be different for each platform - PC, Mobile, Console and so on. Of course it is the same with the price.

The other problem is that for example if the game is for PC platform it has requirements like RAM, Video card and so on. So i decided to make another class for the platformType.

Do you think that the following desing is good?

Here is the main Game object class:

public class Game 
{
   private long systemId;
   private String name;
   private String developer;
   private Date registerDate;
   private ArrayList<GamePlatform> gamePlatforms;
   private GameScore gameScore;
   private GameRank gameRank;
   private ArrayList<String> genre;
}

Here is the GamePlatform class and its subclasses

public abstract class GamePlatform 
{
   private Date releaseDate;
   private int price; 
   private String name;
   private PlatformType platformType;

   public GamePlatform(Date releaseDate, int price, String name, PlatformType  platformType) {

       this.releaseDate = releaseDate;
       this.price = price;
       this.name = name;
       this.platformType = platformType;
   }

   public abstract PlatformType getType();
}

child class 1

public class ConsolePlatform extends GamePlatform
{

   public ConsolePlatform(Date releaseDate, int price, String name, PlatformType platformType) {
       super(releaseDate, price, name, platformType);
   }


   @Override
   public PlatformType getType() 
   {
       return PlatformType.CONSOLE;
   }

}

child class 2

public class MobilePlatform extends GamePlatform 
{

public MobilePlatform(Date releaseDate, int price, String name, PlatformType platformType) 
   {
       super(releaseDate, price, name, platformType);
   }


   @Override
   public PlatformType getType() 
   {
       return PlatformType.MOBILE;
   }

child class 3

public class PCPlatform extends GamePlatform 
{

   private PCRequirements pcRequirements;

   public PCPlatform(Date releaseDate, int price, String name, PlatformType platformType, PCRequirements pcRequirements) 
   {
       super(releaseDate, price, name,platformType);
       this.pcRequirements = pcRequirements;
   }

   public PCRequirements getPcRequirements() {
       return pcRequirements;
   }

   public void setPcRequirements(PCRequirements pcRequirements) {
       this.pcRequirements = pcRequirements;
   }


   @Override
   public PlatformType getType() 
   {
       return PlatformType.PC;
   }

}

And finally i have Enum class for the platform types. Here it is:

public enum PlatformType 
{
   MOBILE
   {   
       @Override
       public String toString(){ return "Mobile";} 
   },
   CONSOLE
   {
       @Override
       public String toString(){ return "Console";} 
   },
   PC
   {
       @Override
       public String toString(){ return "PC";} 
   };    
}
share|improve this question
    
gameScore and gameRank probably belong in GamePlatform, as a specific port may be considered 'bad'. –  Clockwork-Muse Jan 28 '13 at 22:20
    
Actually the boss refused this. When a new preview is made for the game if the realase for some platform is not good it will be mentioned. Actually i have an object GameReview which is a different story for now. –  Joro Seksa Jan 29 '13 at 16:27
add comment

3 Answers

  1. ArrayList<...> reference types should be simply List<...>. See: Effective Java, 2nd edition, Item 52: Refer to objects by their interfaces

    List<GamePlatform> gamePlatforms;
    
  2. I'd consider using BigDecimal instead of int price. It can store numbers like $1.99.

  3. I'd rename PCPlatform to PcPlatform. From Effective Java, 2nd edition, Item 56: Adhere to generally accepted naming conventions:

    While uppercase may be more common, a strong argument can made in favor of capitalizing only the first letter: even if multiple acronyms occur back-to-back, you can still tell where one word starts and the next word ends. Which class name would you rather see, HTTPURL or HttpUrl?

  4. The PlatformType enum seems as a duplication here and if I'm right it's purpose is to use it in switch-case statements. Consider polymorphism here. I'd put a public abstract String getPlatformName() method in the GamePlatform class and remove PlatformType completely. Two might be useful reading:

  5. Constructor parameters should be validated. Does it make sense to create a GamePlatform with null or empty string name? If not, check it and throw a NullPointerException or an IllegalArgumentException. (Effective Java, Second Edition, Item 38: Check parameters for validity)

share|improve this answer
    
If i completelety remove the PlarformType i will have to use the instanceof operator to get the type of the class. Do you think it is better? –  Joro Seksa Jan 27 '13 at 17:40
    
@JoroSeksa: instanceof is very similar to switch-case. Try to replace both with polymorphism. (Feel free to post a new question with that code.) –  palacsint Jan 27 '13 at 20:44
add comment

First read palascint item 4.

If you want to continue using the enum, you should consider to restructure it in the following way. Beside the fact that it is easier to read, is is also easier and less error prone to add new enum values. Furthermore you could easily add an icon, an abbreviation or any other field for your platform.

public enum PlatformType {
    MOBILE("Mobile"), CONSOLE("Console"), PC("PC");

    private String label;

    PlatformType(String label) {
        this.label = label;
    }

    public String toString() {
        return label;
    }
}

In case you want to add more fields, you shouldn't use the toString() but a separate method. See (Effective Java, Second Edition, Item 10) for more details.

share|improve this answer
add comment

Maybe you could move private PCRequirements pcRequirements; to GamePlatform and change it to List<Requirement> requirements. So you don't need to downcast to PCPlatform.

Requirements for Console: Broadband-INet, ...

Requirements for Mobile: Version OS/Browser/JS, ...

share|improve this answer
    
This seems good answer. Do i have to make the Requirements class abstract so i have to subclass with PcRequirements, ConsoleRequirements and MobileRequirements because each one accepts different parameters, right? –  Joro Seksa Jan 27 '13 at 19:26
    
it depends. try a simple interface Requirement. So you can use the implementation OSRequirement for PC and Mobile. –  quartzde Jan 28 '13 at 16:34
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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