5
\$\begingroup\$

I am a newb, and whilst I'm sure my formatting can be improved, I really want to know, how I can re-use my code for this simple program. The objective is simple, football game with a menu, that allows game status to be loaded and saved.

MainmenuTest.java

package mainmenutest;

/**
 *
 * @author Darren Estcourt
 */

import java.util.InputMismatchException;
import java.util.Scanner;


public class MainmenuTest extends premierLeagueClubs{

int clubChoice;

public MainmenuTest(){

//constructor
}


public static void main(String args[]){

    MainmenuTest team = new MainmenuTest();

team.getInput();



  } // end main method

public void getInput(){


boolean exit = false;
int option=0;
while (!exit) {

try{
    Scanner in = new Scanner(System.in);     

    menu();
    System.out.println("\n");
    option = in.nextInt();
} // end try

catch(InputMismatchException e) {

    } // end catch
switch (option) {

    case 1:
    chooseTeam();
    exit = true;    
    break;

    case 2: 
    createProfile();
    exit = true;    
    break;

    case 3:
        DeserializeDemo load = new DeserializeDemo();
    load.Deserialize(clubChoice);
        exit = true;
    break;

    case 4:
    credits();  
    exit = true;
    break;

    case 5:
    exit = true;    
    System.out.println("Goodbye!");
    break;

        case 6:
        SerializeDemo save = new SerializeDemo();
    save.Serialize(clubChoice);
        exit = true;
        break;    

    default:
    System.out.println("Invalid option");


    } // end switch

} // end loop

} // end getInput


public int chooseTeam(){


boolean exit = false;
while (!exit) {
System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");
System.out.println("Please enter an option 1-20 to select a team, or 21 to quit : ");
System.out.println("1. Arsenal");
System.out.println("2. Aston Villa");
System.out.println("3. Bournemouth");
System.out.println("4. Chelsea");
System.out.println("5. Crystal Palace");
System.out.println("6. Everton");
System.out.println("7. Leicester City");
System.out.println("8. Liverpool");
System.out.println("9. Manchester United");
System.out.println("10. Manchester City");
System.out.println("11. Newcastle United");
System.out.println("12. Norwich City");
System.out.println("13. Southampton");
System.out.println("14. Stoke City");
System.out.println("15. Sunderland");
System.out.println("16. Swansea City");
System.out.println("17. Tottenham Hotspur");
System.out.println("18. Watford");
System.out.println("19. West Brom");
System.out.println("20. West Ham United");
System.out.println("\n");
System.out.println("21. Quit");
System.out.println("22. Mainmenu");
try{
    Scanner in = new Scanner(System.in);     


    System.out.println("\n");
    clubChoice = in.nextInt();
} // end try

catch(InputMismatchException e) {

    } // end catch  

switch(clubChoice){ 

    case 1:    
    arsenal();
    exit = true;    
    break;


        case 2:
    astonVilla();
    exit = true;
    break;

    case 3:
    bournemouth();
    exit = true;
    break;  

    case 4:
    chelsea();
    exit = true;
    break;

    case 5:
    crystalPalace();    
    exit = true;    
    break;

    case 6:
    everton();
    exit = true;
    break;

    case 7:
    leicester();
    exit = true;
    break;

    case 8:
    liverpool();
    exit = true;
    break;

    case 9: 
    manchesterUnited();
    exit = true;
    break;

    case 10:
    manchesterCity();
    exit = true;
    break;

        case 11:  
    newcastleUnited();
        exit = true;
    break;

    case 12:
    norwichCity();
        exit = true;
    break;

    case 13:    
    southampton();
        exit = true;
    break;

    case 14:
    stokeCity();
        exit = true;
    break;

    case 15:
    sunderland();
    exit = true;
    break;

    case 16:
    swanseaCity();
    exit = true;
    break;

    case 17:
    tottenhamHotspur();
    exit = true;
    break;

    case 18:
    watford();
    exit = true;
    break;

    case 19:
    westBrom();
    exit = true;
    break;

    case 20:
    westHamUnited();    
    exit = true;    
    break;

    case 21:
    System.out.println("Goodbye!");
        exit = true;
        break;

    case 22:
    getInput();
    exit = true;
    break;

    } // end switch
    } // end loop

return clubChoice;  
} // end chooseTeam






public void createProfile(){


} // end createProfile

public void credits(){
} // end credits


public void menu(){

System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n");
    System.out.println("Created by Darren Estcourt");   
    System.out.println("\n");
    System.out.println("Please choose an option  between 1 - 6 : ");
    System.out.println("\n");
    System.out.println("1. Choose team");
    System.out.println("\n");
    System.out.println("2. Create profile");
    System.out.println("\n");
    System.out.println("3. Load game");
    System.out.println("\n");
    System.out.println("4. Credits");   
    System.out.println("\n");
    System.out.println("5. Quit");
        System.out.println("\n");
        System.out.println("6. Save game");

    System.out.println("\n");
} // end menu




} // end class MainmenuTest

ClubInfo.java

package mainmenutest;

/**
 *
 * @author Darren Estcourt
 */


//import java.util.InputMismatchException;
//import java.util.Scanner;
//import java.io.*;
//import java.util.Arrays;

   public class ClubInfo implements java.io.Serializable{


   public String teamName;
   public String stadium;
   public String division;
   public transient int SSN;
   public int stadiumCapacity;

} // end ClubInfo class

premierLeagueClubs.java

package mainmenutest;

public class premierLeagueClubs{

public void arsenal(){

System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Arsenal!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method arsenal


public void astonVilla(){

System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Aston Villa!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();  
} // end method astonVilla

public void bournemouth(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Bournemouth!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method bournemouth


public void chelsea(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Chelsea!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
}  // end method chelsea

public void crystalPalace(){

 System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Crystal Palace!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method crystalPalace

public void everton(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Everton!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method everton

public void leicester(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Leicester!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method leicester

public void liverpool(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Liverpool!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method liverpool

public void manchesterCity(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Manchester City!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method manchesterCity

public void manchesterUnited(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Manchester United!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method manchesterUnited

public void newcastleUnited(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Newcastle United!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method newcastleUnited

public void norwichCity(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Norwich City!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method norwichCity

public void southampton(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Southampton!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method southampton

public void stokeCity(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Stoke City!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method stokeCity

public void sunderland(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Sunderland!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method sunderland

public void swanseaCity(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Swansea City!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method swanseaCity

public void tottenhamHotspur(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Tottenham Hotspur!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method tottenhamHotspur

public void watford(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of Watford!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method watford

public void westBrom(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of West Bromich Albion!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method westBrom

public void westHamUnited(){

    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");


System.out.println("You are the new manager of West Ham United!");
System.out.println("\n");
System.out.println("Press any key to continue...");
MainmenuTest back = new MainmenuTest();
back.getInput();
} // end method westHamUnited

} // end class premierLeagueClubs

SerializeDemo.java

package mainmenutest;

import java.io.*;

public class SerializeDemo
{
   public void Serialize(int clubChoice)
   {


      ClubInfo club = new ClubInfo();
      ClubInfo club2 = new ClubInfo();
     club.teamName = "Arsenal";
     club.stadium = "Emirates";
     club.division = "Premier League";
      club.SSN = 11122333;
      club.stadiumCapacity = 60000;
      club2.teamName = "Aston Villa";


      try
      {
         FileOutputStream fileOut =
         new FileOutputStream("C:/tmp/club.ser");
         ObjectOutputStream out = new ObjectOutputStream(fileOut);


      //   if (clubChoice == 1){
         out.writeObject(club);
         out.writeObject(club2);
        // out.close();
        // fileOut.close();
       //  } 
      //   else
      //   if (clubChoice == 2){
      //   out.writeObject(club2);
         //out2.close();
         //fileout2.close();
      //   }
      System.out.printf("Serialized data is saved in C:/tmp/club.ser");
      }catch(IOException i)
      {
          i.printStackTrace();
      }


   }
} // end class SerializeDemo

DeserializeDemo.java

package mainmenutest;

/**
 *
 * @author Darren Estcourt
 */
import java.io.*;
public class DeserializeDemo
{
   public void Deserialize(int clubChoice)
   {
      ClubInfo club = null;
      ClubInfo club2 = null;
      try
      {
         FileInputStream fileIn = new FileInputStream("C:/tmp/club.ser");
         ObjectInputStream in = new ObjectInputStream(fileIn);
      //   ObjectInputStream in2 = new ObjectInputStream(fileIn);
      //   if (clubChoice == 1){
         club = (ClubInfo) in.readObject();
         club2 = (ClubInfo) in.readObject();
        // in.close();
        // fileIn.close();
       //  } // end if
        // if (clubChoice == 2){
        // club2 = (ClubInfo) in2.readObject();
        // in2.close();
        // fileIn.close();
       //  } // end if
      }catch(IOException i)
      {
         i.printStackTrace();
         return;
      }catch(ClassNotFoundException c)
      {
         System.out.println("Club class not found");
         c.printStackTrace();
         return;
      }
      System.out.println("Saved game loaded...");
      System.out.println("Name: " + club.teamName);
      System.out.println("Stadium: " + club.stadium);
      System.out.println("Division: " + club.division);
     // System.out.println("SSN: " + club.SSN);
      System.out.println("Stadium Capacity: " + club.stadiumCapacity);
      System.out.println("Name : " + club2.teamName);

    }
}
\$\endgroup\$
0

5 Answers 5

10
\$\begingroup\$

DRY

Let's start with your methods arsenal(), astonVilla() and so on. From the title of the question, I think you're already aware these are problematic. Remember that while at its core your program is going to be made up of branches and loops, high-level languages like Java allow us to write in a way that the source code captures the structure of the task it's achieving. It helps to say in plain English what you actually want to do here:

I want to print a greeting message saying that the player is the new manager of the club they chose.

Your code more closely reflects:

I want to print one of many different greeting messages, depending on which club the player chose.

Like your code, this is technically correct, but is less successful in capturing the structure of what's similar and different between the messages. So trying a more literal interpretation of the first statement, we can write:

public void printGreeting(String clubName) {
    System.out.println("\n\n\n\n\n\n\n\n\n\n\n\n\n\n");

    System.out.println("You are the new manager of " + clubName);
    System.out.println("\n");
    System.out.println("Press any key to continue...");
    MainmenuTest back = new MainmenuTest();
    back.getInput();
}

Now instead of many very similar methods, we have one parameterized method. We've cut out a lot of repetition- all the aspects of the messages which were the same every time were being repeated over and over. This principle of not repeating information is called Don't Repeat Yourself.

SRP

You know what would make your game great? A graphical interface. Well, okay, maybe that's a more than you want to add at the moment. But what about a menu like in many keyboard-controlled games where instead of everything having a number, you have a current selected item in the menu, and you can move up and down, then press enter to select? Or, I dunno, maybe you want to highlight some words in different colours, or have the player type words rather than numbers or translate the game to Spanish.

Maybe (in fact, probably), you'll never want to do any of those things, but the important point to notice is that all of those are to do with how you present the game (get input/output), and none of them have any impact at all on how the game itself works. In reality, the rules and logic of your game are completely separate concerns to how you want to display your game. But in your code, they're so mixed together they'd be extremely difficult to pull apart. Any of the changes I mentioned would require changing almost everything.

So with this in mind, let's look at how we might print our club selection menu:

private String displayClubMenu(String[] clubs) {
    for(int i=0; i<clubs.length;i++) {
        System.out.printlin(i + ". " + clubs[i]);
    }

    Scanner in = new Scanner(System.in);     
    System.out.println("\n");
    choice = in.nextInt();

    return clubs[choice];
}

To this you'd pass an array of names of all your clubs, and it would return the selected club. Notice how this is only concerned with presentation concerns, whereas the part that we've now extracted (the club names) is now completely isolated from presentation. If we changed to a GUI, this method would be changed entirely, but the code which uses it could remain exactly the same.

This idea of each piece of code only being concerned with doing a single thing is called the Single Responsibility Principle. Eventually, you'll find that there are more sophisticated structures that can help you with this (in Java, that will be object-oriented programming), but even making sure a single method isn't mixing many responsibilities is important.


There's probably a lot more things to point out, and I'm sure other answers will. However from my perspective, these are the two key high-level concepts that will help you progress at this point.

\$\endgroup\$
6
\$\begingroup\$

Program Flow

Your program flow is quite peculiar currently... If I may represent how choosing a team is done:

// Start
java mainmenutest.MainmenuTest

// Code
MainmenuTest.main()
    |_ MainmenuTest team = new MainmenuTest()
       team.getInput()
            |_ this.chooseTeam()
                    |_ this.arsenal() // first option
                            |_ MainmenuTest back = new MainmenuTest()
                               back.getInput() // -- new instance???

See, the problem is that your code never goes back to the original instance that was started by the JVM, a new one is created every time instead. If each instance had some state to it, the newer instances will not know about those in the earlier ones, and in the worst-case scenario, you will get a StackOverflowError as the method call stack becomes too deep.

What you should be doing instead is to return from each method, and not to call the previous method from a new instance.

Listing teams

In addition to @Ben Aaronson's suggestion of DRY and SRP, you may want to consider implementating your clubs as a form of enum. For example:

enum Club {
    ARSENAL,
    ASTON_VILLA,
    BOURNEMOUTH,
    // ...
}

Some benefits of an enum are:

  • It clearly defines a set of ordered values with a common identity.
  • It is easy to loop through the values using well-understood methods, such as Club.values().
  • There are efficient Collection classes for enums - EnumSet and EnumMap to facilitate their usage.

Names

Since your code is not adhering to any other style consistently, I will suggest sticking to the Java naming convention:

  • class ClassName
  • private String fieldName
  • public static final String CONSTANT_NAME
  • private void methodName()
  • public static void main(String[] args)

Commenting code

This warrants its own section here. :) Commenting code is usually frowned upon, for good reasons:

  1. If your project is already under source control (e.g. Git, Subversion), access to the older version of your code is only a command away.
  2. It adds unnecessary bulk to the codebase without being useful at all. In other words, readability is affected as it makes scanning the codebase harder.
  3. If you happen to be developing code without syntax highlighting, or if your IDE's syntax highlighting is configured incorrectly, it may be hard to spot what code is supposed to run, and what is not.
  4. You may comment buggy code sometimes, and 'experimental' (but working) code at others. It is hard to tell which is which when you want to tackle a bug, or to continue working on the experimental code.

Therefore, I suggest removing the commented code as early as possible. If you really must leave some of these 'dead' code around temporarily, I will suggest putting them in an unused method clearly annotated with @SuppressWarnings("unused").

Serialization

Your ClubInfo class is missing the serialVersionUID field, which is even more crucial as you are relying on standard Java serialization via ObjectOutputStream. Without this explicitly set, you run the risk of not being able to serialize/deserialize properly when you modify your classes, or even when you switch JDKs/JREs.

While serializing with ObjectOutputStream just works, you may want to consider alternatives such as reading/writing your class's fields as text files using a suitable representation, e.g. plaintext, JSON, XML or YAML. The benefit of this is that users can freely view or even modify these representations outside of your Java application. To go a step further, you may even want to consider widely-adopted serialization frameworks such as Apache Avro, Kryo or Protocol Buffers as these frameworks tend to offer both speed and compression compared to ObjectOutputStream.

\$\endgroup\$
3
\$\begingroup\$

Naming

Your choice of names go against conventional naming. Classes should be all UpperCamelCase: PremierLegueClubs, MainMenuTest. Similarly methods in java are pascelCase serialize(), deserialize().

Comments

You'll have a heated debate about if you should comment or not. I've heard two philosophies on the matter: Code should speak for itself; Comments should help explain to users of your code how to use it. Both groups of people would agree that putting a comment at the end of a method/class to tell you its the end of a specific method/class is bad form. (It screams powershell user to me). They don't serve a purpose but to throw a red flag to others that you feel the code is getting out of control and is too difficult to read. That is a gut feeling too that should not be ignored! There are a few ways to make code easier to read. First would be as was suggested earlier (I believe by you) the formatting. I know for sure Netbeans, Eclipse and IntelliJ all have a auto format ability to code (IMO Netbeans has the better of the 3, but I currently use IntelliJ and it does a good job). Use that! It doesn't hurt when you are finished to minimize all your methods and see if you have them evenly spaced apart. Some of your classes have multiple spaces between methods or 1 space. Be consistant.

Public Fields

It has been agreed that this is not a good idea. Immutable classes are the easiest to test, and easiest to trust because you know that they can't be changed. That being said, consider ClubInfo. it has public fields. seeing as you only use it in your deserialize and serialize demo code this would probably be ok as a scratch pad, but consider using getters and setters on the concept of the data (notice I said concept and not data). I like the FuelTank example best to explain this. You could make the fuel tank have the fields TotalCapacity and RemainingCapacity public. And in your program you could do

public void useSomeGas{
    tank.RemainingCapacity -= 2;
    gasGuage = tank.RemainingCapacity/tank.TotalCapacity;
}

problem with this is taht anyone can change TotalCapacity. also why does this class care how to calculate remaining fuel. Your concept here is to use some gas, and then get the percentage left. so make those methods in the FuelTank class and don't expose your fields. (To see the point even more ask around and see how many people know the exact figure of fuel that their fuel tank holds)

Unit tests

I think the best way to make your code reusable is to write unit tests. How/Why? When you write tests it gives you a safety net to go in and change things without the worry of breaking how the logic works. Also it points out areas in your code that are pain points quickly. Above is mentioned that changing how you display this game would be a HUGE pain point. Lets imagine for a minute that your game used a GUI instead of the console, AND I started writing unit tests to show that the strings are lining up correctly. Are you going to expose all your TextBoxes so I can getText() from them? Yuck! then when you go to change them because of your test you'll end up throwing something (physically or metaphorically). Normally the first thing to get thrown is the unit tests. But dont! Lets consider your DeserializeDemo and SerializeDemo code. Those two bits are prime for unit tests (as they can easily be tested as they are). You'll also get the benefit of not having to run your code to test if they work. This post isn't to tell you how to write unit tests.. There are countless blogs/videos/demos out there on how to do that already!

So here is my test. Simple and to the point.

@Test
public void testSerialization() throws Exception {
    SerializeDemo serializer = new SerializeDemo();
    serializer.Serialize(1);
    
    DeserializeDemo deserializer = new DeserializeDemo();
    deserializer.Deserialize(1);
}

overal that is a bad test. Mainly because i'm not asserting anything nor am I checking that what was serialized is what was deserialized. That's cool we can work with that. So now lets write my test the way I want my code to work. I want to take a clubinfo, serialize it, then using my deserializer I want to check that it gave back the same values.

@Test
public void correctTestForSerialization() throws Exception{
    ClubInfo club = new ClubInfo();
    club.division = "division";
    club.SSN = 1234;
    club.stadium = "stadium";
    club.stadiumCapacity = 2;
    club.teamName = "teamName";
    SerializeDemo serializer = new SerializeDemo();
    serializer.Serialize(club);

    DeserializeDemo deserializer = new DeserializeDemo();
    ClubInfo deserializedClub = deserializer.Deserialize();
    
    assertEquals(deserializedClub.division, club.division);
    assertEquals(deserializedClub.SSN, club.SSN);
    assertEquals(deserializedClub.stadium, club.stadium);
    assertEquals(deserializedClub.stadiumCapacity, club.stadiumCapacity);
    assertEquals(deserializedClub.teamName, club.teamName);
}

still not the best test in the world, but it would serve its purpose very well. I could now change SerializeDemo and DeserializeDemo to work differently and as often as I wanted. As long as that test still passed I know that it works as I want it to. Also that test is fast. The test above runs in 37ms. It would be impossible for you to load up your program and test that that quickly.

\$\endgroup\$
3
  • \$\begingroup\$ why should the tank care about how to compute the value to be displayed to the user? In this case it is percents, but the gauge could be 5 bars, or 3 section of varying size. I disagree that it is the responsability of the tank to compute that information. It is its responsability, however, to figure out what to do with the fact that a given qty of fuel was used (possibly some tanks are made of several compartements, ...) \$\endgroup\$
    – njzk2
    Commented Sep 24, 2015 at 17:53
  • \$\begingroup\$ @njzk2 that is debatable. The tank has a tool (usually a float of sorts) which does the actual fuel measurement. One could argue that you should query that object, but the wires come out of the tank and the tank is sealed. How a device decides to show a percentage is up to it. \$\endgroup\$ Commented Sep 24, 2015 at 18:31
  • \$\begingroup\$ The part I don't like is the percentage. I guess one thing that would be required is a sort of driver for the tank to read the raw value and output it in a standardize way. In fact percentage is probably good for that. \$\endgroup\$
    – njzk2
    Commented Sep 24, 2015 at 19:20
0
\$\begingroup\$

When coding, remember to avoid repetition, @Ben Aaronson already gave you one advice, use functions, I now suggest that you use loops to reduce repetition even more:

System.out.println("1. Arsenal");
System.out.println("2. Aston Villa");
System.out.println("3. Bournemouth");
System.out.println("4. Chelsea");
System.out.println("5. Crystal Palace");
System.out.println("6. Everton");
System.out.println("7. Leicester City");
System.out.println("8. Liverpool");
System.out.println("9. Manchester United");
System.out.println("10. Manchester City");
System.out.println("11. Newcastle United");
System.out.println("12. Norwich City");
System.out.println("13. Southampton");
System.out.println("14. Stoke City");
System.out.println("15. Sunderland");
System.out.println("16. Swansea City");
System.out.println("17. Tottenham Hotspur");
System.out.println("18. Watford");
System.out.println("19. West Brom");
System.out.println("20. West Ham United");

Should become, in pseudocode:

for index, name in enumerate({ "Arsenal", "Aston Villa", ...}) {
    System.out.println((index + 1) + name);
}

The list will only take two or three lines, and voilà, you saved 15 lines of code :)

\$\endgroup\$
7
  • 1
    \$\begingroup\$ If you're putting all 15 lines in one (I mean, all the 20 elements in the {} enumeration object), and have a horribly long one-liner, I'd say you're not saving anything. \$\endgroup\$ Commented Sep 24, 2015 at 12:34
  • \$\begingroup\$ @PierreArlaud you may use 2, 3, 4 or more lines to spread the array on, as it pleases you. The number is also automatically generated \$\endgroup\$
    – Caridorc
    Commented Sep 24, 2015 at 13:00
  • \$\begingroup\$ I'd personally use as many lines as there are football clubs, because it makes more sense than arbitrarily grouping them by 3 or 4 per line, so the point is there is no line saving. \$\endgroup\$ Commented Sep 24, 2015 at 13:12
  • 1
    \$\begingroup\$ @PierreArlaud You now want to add a team at position 5. You must manually increment the index for all teams after position 5. With my method, you just add it \$\endgroup\$
    – Caridorc
    Commented Sep 24, 2015 at 13:38
  • \$\begingroup\$ I only criticized your idea of "saving lines". Your answer only mention "reducing repetition" and "saving 15 lines of code", when — while true — it's actually about code evolution and maintenability. \$\endgroup\$ Commented Sep 24, 2015 at 14:35
0
\$\begingroup\$

TL;DR

If you want reusability, in a nutshell:

Don't have a single club name in your code.

Not in strings, not in constants, not in class, variable or method names.

Get it as parameter. Either from a configuration file, from the command line, or from another program that has the sole responsability of providing the club names.

\$\endgroup\$

Your Answer

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

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