5
\$\begingroup\$

I gave myself a small project to work on and at first I wanted to use the MVC approach, but I got confused as to which part of my code should go into which class. I did some research and still don't quite understand how I should go about it.

This is the GUI I made:

Graphical User Interface


I created the View class:

public class LevelUpView extends JApplet{

    Image levelUp, clickMeImg, expBarImg, currentLvlImg, obtainedExpImg;
    Image expBarImg2, expBarImg3, expBarImg4;
    ImageIcon levelUpIcon, clickMeIcon, expBarIcon, currentLvlIcon, obtainedExpIcon;
    ImageIcon expBarIcon2, expBarIcon3, expBarIcon4;
    JLabel title, expBar, currentLvl, currentLevel, obtainedExp;
    JLabel expTotal;
    JButton clickMe;
    JPanel mainPanel;

    int experience = 0;
    int level = 0;
    String lvl = "0";
    String exp = "0";


    public void init(){

        mainPanel = new JPanel( new BorderLayout()); // main container for layout 
        mainPanel.setBackground(new Color(213, 213, 210));
        add(mainPanel);
        createTitle(); // creates each section of the layout
        createCenter();
        createBottom();
    }

    private void createTitle(){ 

        levelUp = getImage(getCodeBase(), "Images/LevelUpLogo.png");
        levelUpIcon = new ImageIcon(levelUp);
        title = new JLabel(levelUpIcon);
        mainPanel.add(title, BorderLayout.NORTH);
    }

    private void createCenter(){ 

        JPanel centerPanel = new JPanel();
        centerPanel.setLayout( new BoxLayout( centerPanel, BoxLayout.Y_AXIS));
        centerPanel.setBorder(new EmptyBorder(0,0,-140,0)); // spacing 
        JPanel clickMeLayout = new JPanel(new BorderLayout());
        clickMeLayout.setBorder( new EmptyBorder(90,0,0,0));
        JPanel expBarLayout = new JPanel( new BorderLayout());
        expBarLayout.setBorder( new EmptyBorder(-190,1,1,55));
        expBarLayout.setOpaque(false); // allows background to show 
        clickMeLayout.setOpaque(false);
        centerPanel.setOpaque(false);

        clickMeImg = getImage(getCodeBase(), "Images/ClickMe.png"); // getting Images 
        expBarImg = getImage(getCodeBase(), "Images/EXPBar.png");
        expBarImg2 = getImage(getCodeBase(), "Images/EXPBar2.png"); 
        expBarImg3 = getImage(getCodeBase(), "Images/EXPBar3.png");
        expBarImg4 = getImage(getCodeBase(), "IMages/EXPBar4.png");

        clickMeIcon = new ImageIcon(clickMeImg);
        expBarIcon = new ImageIcon(expBarImg);
        expBarIcon2 = new ImageIcon(expBarImg2);
        expBarIcon3 = new ImageIcon(expBarImg3);
        expBarIcon4 = new ImageIcon(expBarImg4);

        clickMe = new JButton(clickMeIcon);
        clickMe.setBorderPainted(false);
        expBar = new JLabel(expBarIcon);

        clickMeLayout.add(clickMe, BorderLayout.NORTH); 
        expBarLayout.add(expBar, BorderLayout.NORTH);
        centerPanel.add(clickMeLayout);
        centerPanel.add(expBarLayout);
        mainPanel.add(centerPanel, BorderLayout.CENTER);
    }

    private void createBottom(){

        Font numberFont = new Font("Monospaced", Font.BOLD, 25); // font for exp amount and level 

        JPanel bottomPanel = new JPanel( new GridLayout(2,2));
        JPanel bottomFlowPanel = new JPanel( new FlowLayout());
        bottomPanel.setOpaque(false);
        bottomFlowPanel.setOpaque(false);

        currentLvlImg = getImage( getCodeBase(), "Images/CurrentLevel.png");
        obtainedExpImg = getImage(getCodeBase(), "Images/EXPObtained.png");

        currentLvlIcon = new ImageIcon(currentLvlImg);
        obtainedExpIcon = new ImageIcon(obtainedExpImg);

        currentLvl = new JLabel(currentLvlIcon);
        obtainedExp = new JLabel(obtainedExpIcon);
        expTotal = new JLabel(exp);
        currentLevel = new JLabel(lvl);
        expTotal.setFont(numberFont);
        currentLevel.setFont(numberFont);

        bottomPanel.add(currentLvl);
        bottomPanel.add(currentLevel);
        bottomPanel.add(obtainedExp);
        bottomPanel.add(expTotal);
        bottomFlowPanel.add(bottomPanel);
        bottomFlowPanel.setBorder(new EmptyBorder(0,255,0,0));
        mainPanel.add(bottomFlowPanel, BorderLayout.SOUTH);
    }

}

And I ended up just doing the rest in the Model class:

public class LevelUpModel extends LevelUpView implements ActionListener{


    private int expForLevels[] = { 20, 50, 100, 160, 240, 340, 420, 560, 750, 999};

    public void init(){
        super.init();
        clickMe.addActionListener(this);
    }

    public void actionPerformed(ActionEvent ae) {

        Object source = ae.getSource();

        if( source == clickMe){
            experience++;
            exp = "" + experience;
            expTotal.setText(exp);
            levels();
        }
    }

    private void levels(){ // as the user gains exp and levels up

        int amount = Integer.parseInt(exp);

        if ( amount == (expForLevels[0]/3)){ 
            expBar.setIcon(expBarIcon2); // changes the expBar image 
        }
            else if( amount == ((expForLevels[0]/3) + 6)){ 
                expBar.setIcon(expBarIcon3);
            }
            else if( amount == (expForLevels[0] - 2)){ 
                expBar.setIcon(expBarIcon4);
            }
            else if( amount == expForLevels[0]){
                levelUp();
                mainPanel.setBackground( new Color(58, 144, 255)); // dark blue 
            }

        if( amount == expForLevels[1] - 20){
            expBar.setIcon(expBarIcon2);
        }
            else if( amount == expForLevels[1]-10){
                expBar.setIcon(expBarIcon3);
            }
            else if( amount == expForLevels[1] - 2){
                expBar.setIcon(expBarIcon4);
            }
            else if( amount == expForLevels[1]){
                levelUp();
                mainPanel.setBackground(new Color(255, 133, 0)); // burnt orange 
            }

        if( amount == expForLevels[2] - 35){
            expBar.setIcon(expBarIcon2);
        }
            else if ( amount == expForLevels[2] - 20){
                expBar.setIcon(expBarIcon3);
            }
            else if( amount == expForLevels[2] - 5){
                expBar.setIcon(expBarIcon4);
            }
            else if( amount == expForLevels[2]){
                levelUp();
                mainPanel.setBackground(new Color(0, 178, 18)); //  dark green
            }

        // same thing will be done up to level 10
    }

    private void levelUp(){ // each time there's a level up
            level++;
            lvl = "" + level;
            currentLevel.setText(lvl);
            expBar.setIcon(expBarIcon);
    }

} 

Once the button is clicked experience is added and the experience bar changes as it gets closer to the next level up. Once you level up, the background changes and the bar is reset to its original image. (Up to level 10).

I realized I was doing this all wrong and the best way to go about it was to get my code looked at. Also, I'm confused as to which variables should be made private?

So, what I'm asking is for an example of which part of my code should go into which class, considering I made a controller class as well. Also, an example of using private variables within my code. As well as any more problems, I need all the constructive criticism I can get.

As a beginner, I want to learn better practices as soon as I can!

\$\endgroup\$
3
  • 2
    \$\begingroup\$ Why code an Applet in the first place? If it is due to the teacher specifying it, please refer them to Why CS teachers should stop teaching Java applets. \$\endgroup\$ Sep 6, 2016 at 16:45
  • 2
    \$\begingroup\$ Java was my introductory language & through the semester we only worked with Applets, reason being so we could put them online. The post you referred me too @Simon has me questioning that method now.. but regardless I can familiarize myself with using frames now. Thanks! \$\endgroup\$
    – zalaniz2
    Sep 6, 2016 at 22:04
  • \$\begingroup\$ @zalaniz2 If you want to use Java and put stuff online, you should consider looking into Google Web Toolkit. As an example, see Memory with a twist (an old Code Review question of mine) \$\endgroup\$ Sep 7, 2016 at 10:53

1 Answer 1

4
\$\begingroup\$

Some short note without really reading into the source.

In MVC usually the Model is independent of View and Controller.

So your public class LevelUpModel extends LevelUpView implements ActionListener looks like mixing together Model and View class, supplementing functionality of Controller too.

Some general notes how I would split these:

Model:

  • contains experience, level (private, plus getter/setter), level exp-threshold values table (may be public constant, if it's not only internal, like some part of app is displaying page with exp levels)
  • may contain some further model related constants, like MAX_EXP_TO_ADD, etc.
  • contains "business" logic operating with the model state

example of API:

public interface LevelUpModel {
    // it's actually quite rare for model class
    // to extend some non-model/non-trivial class

    // *** getters/setters ***

    int getExperience();
    // no need for setter? (addExperience method available to adjust exp)
    int getLevel();
    // no need for setter? (addExperience method available to adjust level)
    int getLevelUpExperience(); // returns required exp to level up (at current level)

    // *** biz-logic ***
    void reset(); // resets everything to be prepared for new game. Or "init"
    boolean addExperience(int addExp); // returns true upon levelling up

}

View:

I prefer to couple my views tightly with models, so the init/update methods take reference to actual model class (ie. View has to import Model).

It's not storing the model reference, only using it's API to init/update it's current state and forgetting about it, so you can any time change the View to display current state of different instance of model.

With the above model, and intent to have special effect for "level up" event, I would probably design API as:

void update(final LevelUpModel model, final boolean leveledUp) { ... }

Which would use getters to set up the exp bar, exp number display, level number display, set up correct colours based on those, and finally to start some effect when leveledUp would be true (having that "effect" as an stand-alone app entity, capable to handle it's own life cycle, like some animation of fire works or sound player playing ding once).

In case of some more complex level-up effect I would consider either making state of it part of original model, or having separate LevelUpEffectModel to hold state of effect, with it's own view and controller.

Controller:

Finally controller is the glue of view and model, but should not contain any biz-logic, or low-level view updating commands.

So in this example it would do probably things like:

  • contains the LevelUpModel instance, and View instance. Either receives it from global app initializer (such design can be seen as "dependency injection", helping with unit testing), or creates the instances in it's own init().
  • calls reset/init upon them at the beginning (in Controller.init(model, view) ?)
  • gives an on-button-click handler to View
  • the on-click handler will do model.addExperience(some_value), and view.update(model, true/false) (in more complex multi-threaded app it may also resolve any race conditions, in case model itself is not thread-safe designed).
  • probably that's all?

So things like setIcon belongs to View.update, ++exp; belong to Model.addExperience, poking model/view in reaction to the click event belongs to Controller (although it shouldn't contain the logic itself, ie deciding whether level up happened or not, that's responsibility of Model).

Model contains every important state of the world, so the view can be reconstructed out of it (above I'm breaking it a bit by that "leveledUp" value, writing it now, I would probably move it inside Model, and create one more getter boolean didLevelUp(); being updated by last addExp call) - from scratch.

View contains everything what is displayed, for example it contains the string representation of experience (Model doesn't have to, as it can be constructed from the int in Model, by "formatter" in View). And also contains logic, how to display it.

Controller should only "connect the dots" and control the execution flow, when what should be called/updated.


The Model/View relation - how I like it - can be demonstrated on this classic:

Consider having blog application, so for each article you have date+time of publishing the article.

Then Model should contain UTC timestamp value (can be unix timestamp, if you need only dates since 1970 onward).

And View will do all the formatting magic, ie showing "5 seconds ago" for fresh article, or "previous millennium" for some really old article, also converting the date/time to local time zone of user (source data stored on server being in UTC, time zone agnostic).


Usually when you can UnitTest all model API easily in pure JUnit (without mocking JApplet environment or providing fake UI/display), you are on the right track. There's sometimes need to provide some complex back-end functionality, like database, or server API, for model classes, but it should be still quite easy to mock only a bit of it, and having model exercised to do everything it should.

Testing View classes is quite often much more difficult, or straight counter-productive, if you change the design a lot, so your tests would have to be updated often.

Testing Controller code depends, usually needs some good instrumentation helper to simulate all kind of inputs/events, mock views (just verifying the update is called at proper places), and often may work upon real model classes, making such tests more integration than unit.

Why I'm talking about tests so much, while you didn't even ask. Because once you do the MVC properly, unit testing "model" part becomes so easy, that it's low hanging fruit and should be probably your next topic to try out, after you get more comfortable with MVC patterns. :)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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