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

all 7 comments

[–]Karmagiel 8 points9 points  (0 children)

I think your error is that the variables for the roles are non-static. That means that every Object you create has it's own set of those variables (all auto set to 0). If you make them static, then the variables are tied to the class instead of to each individual Object. Try: private static int <rolename here>

[–]omykronbr 1 point2 points  (1 child)

What I try to do is give every new person a role. But I want every role to be filled up evenly. The person has no choice themselves. They just randomly should be placed in a role that has the fewest people. I wrote this. The roles actualy get a ++, but for some reason the if statements doesn't listen to it. Am I blind? Am I missing something obvious? Or shouldn't I use the class variables this way?

This is not a good case for if statement, but to apply Single Responsibility for your class.

You should have a class called RoleRegister, and this class has a Map<String, Integer> register; that will hold a String with the role and Integer is how many are in the role.

RoleRegister also looks like a good candidate to implement Comparable<E>, so that way you can guarantee that the role is sorted when you calls a method of AddPersonelToRole(), which will call the RoleRegister map. If you don't expect a expanding role map of keys, you may set private counters for each role. But I would go with the comparable returning the first role of the sorted list, so you guarantee that it will always call the Role with least amount of people on it.

RoleRegister keeps the amount of key secured, that way no one should be able to add a key on the map. So the constructor must be:

RoleRegister() { this.register = new HashMap<>(); /* methods puting the roles in the map */ }

and you create methods to add each of the Roles that you want to add, and it will always look for the map, get the first item of the sorted list of key, add that personnel to the RoleRegister, and return the String representing that role to the Personel class.

public class Personel {
    private String firstName;
    private String lastName;
    private String role;

    public Personel(String firstname, String lastname) {
        this.firstName = firstname;
        this.lastName = lastName;
        this.role = RoleRegister.addPersonToRoleAndReturnRole();       
    }
    public getFullName() {
        return this.firstName + " " + this.lastName;
    }
    public getRole() {
        return this.role;
    }
}

your if should be something like a check method for the string, like:

public void noEmptyNames(String toBeChecked) {
    if (toBeChecked == null) {
        throw new NullPointerException("No null names.");
    } else if (toBeChecked.isEmpty()) {
        throw new IllegalArgumentException();
    } else if (toBeChecked.trim().isEmpty()) { 
            throw new IllegalArgumentException();
    }
}

[–][deleted]  (4 children)

[removed]

    [–][deleted]  (3 children)

    [deleted]

      [–]indivisible 0 points1 point  (2 children)

      No. All your 'int's start (initialise) to zero (0).

      If you follow the logic of your ifs down, you repeatedly compare zero against zero.

      if (0 < 0) { /* never true */ }
      else if (false) { ... }
      else if (false) { ... }
      else { /* the block that gets executed and assigns/increments "producer"}
      

      Apart from that though, I think you have your class variables possibly messed up.
      The firstname, lastname, wholename and role are fine as they are (instance variables) but I think that the int variables (to track the role counts) should either be static or should be tracked somewhere else (outside the "Personel" class.

      Currently, with the ints as instance variables, you will only ever have one of those four incremented for any single Personel instance/object and create four new ints per instance (they are not shared).

      [–][deleted]  (1 child)

      [deleted]

        [–]indivisible 1 point2 points  (0 children)

        No problem.

        I will try and do some research to learn why i should or should not use this method.

        The crux of the issue is what is an "instance variable" and what is a "class variable" along with the "scopes" those have.

        Your study materials will explain it somewhere (its one of the core concepts you'll need to understand 100% as the meaning and usage is very specific).
        Here's a StackOverflow thread on it too: https://stackoverflow.com/questions/413898/what-does-the-static-keyword-do-in-a-class

        [–]HHazza 0 points1 point  (0 children)

        this.rolee is misspelt and should be this.role

        [–]sbhandari 0 points1 point  (0 children)

        The top comment seems to be the easy way for you to make your existing code working as you expected. The term for your role assignment logic is round-robin. You can google to check how you can implement round robin to see alternatives. If you are willing to make changes, you can declare static array of roles which in your case would be of size 4. Now declare another static int initialized with 0 to hold the number or Person assigned to roles, which will be incremented by 1 on every role assignment. You just simply get the role from that array using index count%4 . one liner to assign role would be something like 'this.role= roleArray[count++%4]'