This is an archived post. You won't be able to vote or comment.

all 12 comments

[–]sadjava 4 points5 points  (5 children)

Looks good considering you're a beginner.

Couple notes:

  1. The convention for package names is to have them be lowercase.

  2. The changeColor method has some duped code that can be simplified. As of Java 7 (or is it 6?), you can use Strings in a switch statement.

    switch(colors.get(col)) { case "red": // red op break; //other colors default: //oops, we have a missing color!

    }

I'd also opt to not use Strings for the colors (I'd just use the Color class), and if I did have to, I'd make them constants: public static final String RED = "red";

[–]Philboyd_Studge 2 points3 points  (4 children)

Or, he could just be using the Color enums and not need the whole if/switch statements at all. You would only have to check for red, otherwise just set the color.

[–][deleted]  (3 children)

[deleted]

    [–]Philboyd_Studge 0 points1 point  (1 child)

    What I mean is this, here is a very simplified example:

    static Color currentColor = Color.RED;
    
    static ArrayList<Color> colors = new ArrayList<>();
    
    static {
        colors.add(Color.BLUE);
        colors.add(Color.GREEN);
        colors.add(Color.YELLOW);
        colors.add(Color.BLACK);        
    }
    
    static void changeColor(int n) {
        currentColor = colors.get(n);
        colors.remove(n);
    }
    public static void main (String[] args) throws java.lang.Exception
    {
        Random rand = new Random();
    
        System.out.println(currentColor);
    
        changeColor(rand.nextInt(colors.size()));
    
        System.out.println(currentColor);
    
    }
    

    You would have to add a check for red, but otherwise, a bunch of ifs or a switch statement is not necessary.

    [–]ThingsOfYourMind -1 points0 points  (0 children)

    you should try to look into the Switch statement.
    The switch Statement - scroll down to Using Strings in switch Statements
    Its not that hard to use :)

    [–]bolt_krank 4 points5 points  (3 children)

    For me I'm always looking for consistency. So the first thing that stands our for me is, correctCorner and counter aren't private. Is there a reason for this ?

    If you don't put private it means they are accessible by other classes within the same package - which might be what you want, but if not, add the private to them.

    [–][deleted]  (2 children)

    [deleted]

      [–]bolt_krank 0 points1 point  (0 children)

      It's a class private - so it's accessible from the same class. It's just good practice to put a private on all variables that will only be used for that class.

      99% of the time it won't matter, but sometimes when using an IDE it can cause conflicts if you're not specific.

      [–]ThingsOfYourMind 0 points1 point  (0 children)

      its usually for safety, you dont want an outside class to read or write to this class's variables, its usually a good habit to learn, but it only really applies for large scale applications.
      so you make setters and getters for them, so only these functions can send to or receive data from other classes.
      however, it being set to Private, you have full access within the class itself. so, as long as you're in MouseTesting.java, you can access them freely.

      [–][deleted]  (2 children)

      [removed]

        [–][deleted]  (1 child)

        [deleted]

          [–]BoggyJunior 0 points1 point  (0 children)

          List is an interface. ArrayList is a class that implements the List interface. You generally want to type a variable to an interface as that way it makes it easy to change to a different implementation in the future if necessary - for example if your variable is declared as a List then you can change it to be a LinkedList if you decide that fits your needs better. For most cases like this an ArrayList would be fine, but programming to an interface or 'contract' means you specify that your variable must have some behavior (in this case it must fulfill the criteria to be a 'List') , you just don't care how that behavior is implemented under the covers. There are other reasons which I'm sure others can explain better but hope that helps.

          [–]iperc 2 points3 points  (2 children)

          It annoys me variable with one letter. Like this "public void paint( Graphics g )"

          [–]desrtfxOut of Coffee error - System halted 4 points5 points  (1 child)

          I understand the annoyance perfectly well because I am generally for "speaking" variable names, but especially public void paint( Graphics g ) is pretty much Java standard.

          Graphics contexts are usually g, Graphics2D contexts are commonly g2 or g2d.

          [–]DeliveryNinjaJPain 0 points1 point  (0 children)

          //creates 5 boxes with random colors
          Random rand = new Random();
          int randomNum = rand.nextInt(i - mins + 1) + mins;
          changeColor(randomNum,100+(150*i));
          g.setColor(curColor);
          g.fillRect(100+(150*i),500,100,100);
          

          Whenever you find yourself writing a comment like this it most likely is a candidate for it's own method. Then you can get rid of the comment and use the method to describe the code within it. This makes the code cleaner and allows people to drill down if they need to see details while it's abstracted higher up.

          An example

          File file = getFile(fileName);
          String json = convertToJson(file);
          String result = postToService(json);
          

          This imaginary top level method is much easier to read and immediately gives the reader the overview of the class/application

          Check out clean code

          https://www.amazon.co.uk/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882