CODEWITHSUNDEEP
javaarraysarraylistcontains
Shahram
Shahram

Reputation: 61

Java - ArrayList not detecting duplicate value

I have written the following class for my program:

public class RefGen {

    private static String refNo;

    protected static void generate(){

         //Create array to store reference
         ArrayList<String> refList = new ArrayList<>();

         //Get date and set output format
         DateFormat dateFormat = new SimpleDateFormat("yyMMdd");        
         Date curDate = new Date();

          //Variables
          String clientKey = InGenUI.clientText.getText();
          String refDate = dateFormat.format(curDate); 
          String refType = InGenUI.typeCombo.getSelectedItem().toString();
          String userName = InGenUI.userCombo.getSelectedItem().toString();
          String ref;
          int n = 1;

          //Create Reference
          refNo = clientKey.toUpperCase() + "/" + refDate + "/" + refType + "/" + userName + "/" + Integer.toString(n);

          //Check to see if refNo already exists in array
          while (refList.contains(refNo)) {
              n = n + 1;
              refNo = clientKey.toUpperCase() + "/" + refDate + "/" + refType + "/" + userName + "/" + Integer.toString(n);
              refList.add(refNo);
          }

          refList.add(refNo);
          System.out.println(refList);
      }

      public static String reference(){

          return refNo;
      }
  }

The purpose of this class is to generate a unique reference number and store it in an array. Before doing so, it needs to check to see if the array already contains the value. If not, n is incremented by 1 until refNo becomes a unique value that does not exist in the array.

RefGen.reference() is called by genButton in InGenUI.java which output's refNo's value to clientLabel also in InGenUI.java:

    private void genButtonActionPerformed(java.awt.event.ActionEvent evt) {                                          
        RefGen.generate();
        String refNo = RefGen.reference();
        clientLabel.setText(refNo);
    }

The program generates the reference number but never increments the label value in InGenUI.java or the actually array itself in RefGen.java. It also seems that the array only holds a single value on each button click.

I think that refList stores the original refNo value generated but empties the array each time it does so. I suspect that each time I click genButton I am actually creating a new instance of refList and hence wiping out the old values. Is this correct? If so, how can I protect the instance of refList I created while keeping it in the RefGen.java class?

Thank you in advance.

Upvotes: 2

Views: 146

Answers (2)

Thomas
Thomas

Reputation: 88707

Well, you could make refList a class variable, i.e. move it out of the generate() method.

I don't know what exactly you mean with "protect the list" but you can make the list a private/protected member and only allow manipulation via special methods.

Additionally, I'm not sure this part works as intended (neither currently nor after the suggested changes):

while (refList.contains(refNo)) {
          n = n + 1;
          refNo = clientKey.toUpperCase() + "/" + refDate + "/" + refType + "/" + userName + "/" + Integer.toString(n);
          refList.add(refNo); //<-- problems here
 }

Currently, your List won't contain refNo and thus it won't be added.
After the suggested changes the list would contain refNo but now you'd add it again. So you'd probably want to remove that add operation.

As already suggested you also might want to use a Set<String> instead and choose an implementation that fits your needs, e.g. TreeSet<String> of the reference numbers should be sorted of LinkedHashSet<String> if the order of (first) insertion does matter.

Using a set you could even keep re-adding the elements (it's still odd, but you could) since sets don't allow for duplicates.

You could also change your code to try and add a reference number and it that didn't succeed or the size of the set didn't change you increment the number and try again.

UPDATE:

I didn't realize that the method was static (thanks Stephen C), so I moved the parts concerning instance variables to here.

Generally static variables aren't that well recieved for storing data such as your reference list. There are multiple reasons, e.g. what if you'd want to use the same code for multiple lists, encapsulation and inheritance (e.g. you can't override static methods and having non-static methods write to static variables often isn't really meaningful), the possibility of memory leaks (if you don't manually set statics to null or unload classes yourself, they can't be collected) etc.

Thus you might consider changing two possibilities for changing your code:

  • keep the list outside of the class and pass it as a parameter
  • change the code to make the list and generate method non-static.

Upvotes: 1

Juned Ahsan
Juned Ahsan

Reputation: 68715

You seems to be creating a fresh array list every time you call generate:

     //Create array to store reference
     ArrayList<String> refList = new ArrayList<>();

You should declare it outside the method at class level.

Upvotes: 5

Related Questions