Solved Configuration File Bug - How to Remove a String From a String List?

Discussion in 'Plugin Development' started by Googlelover1234, Jun 14, 2014.

Thread Status:
Not open for further replies.
  1. Offline

    Googlelover1234

    Hey, I have a bug in my chat plugin. I was finishing it up, and I had just added a command to remove a word from the config file. It keeps acting like it is not there, and does not remove it. Any help would be appreciated!

    Code:

    Code:java
    1. if (cmd.getName().equalsIgnoreCase("antiswear")) {
    2. if (args.length == 2) {
    3. if (!sender.hasPermission("antiswear.remove")) {
    4. sender.sendMessage("No permission!");
    5. } else {
    6. if (args[0].equalsIgnoreCase("remove")) {
    7.  
    8. List<String> words = this.getConfig().getStringList(
    9. "Settings.words");
    10.  
    11. if (words.contains(args[1].toLowerCase())) {
    12. sender.sendMessage(args[1]
    13. + " is not a banned word!");
    14. } else {
    15.  
    16. words.remove(args[1]);
    17. this.getConfig().set("Settings.words", words);
    18. sender.sendMessage(args[1]
    19. + " was removed from the banned words.");
    20. this.saveConfig();
    21. this.reloadConfig();
    22.  
    23. }
    24. }
    25.  
    26. }
    27. }
    28. }
     
  2. Offline

    garbagemule

    Take a good, long look at line 11 :)

    Besides that, you may want to try to be more consistent with converting strings to lowercase or not. In line 11 you are converting the argument to lowercase, but in line 16 you're trying to remove the original argument. If the word in the list is not lowercase, or if the argument isn't lowercase, you will get inconsistent behavior. One way around this would be to extract the argument and store it in a local variable and replace all occurrences of your array access with that variable. This will help you keep your code consistent.

    Finally, you may want to check the return type of the remove() method and read the documentation for it. Use the return value instead of checking for membership first.
     
  3. Offline

    Googlelover1234

    Ah, I see what you're saying. Let me try something, and I'll get back to you. Thanks for responding.
     
  4. Offline

    bdubz4552

    Googlelover1234 Yes, make sure cases match. You need to .toLowerCase() any time you mention args[1]. Also, you do need to take a long hard look at lines 11 and 16. Line 11 is saying that if the String list contains args[1], execute x y z. Line 16 is inside line 11's else statement. Meaning, if the String list does NOT contain args[1], then line 16 and below is executed. So, basically in that else statement, you're telling your plugin "because args[1] is not in the list, I want you to remove it". A little food for thought.
     
  5. Offline

    Googlelover1234


    Perfect, thanks for noticing the lowercase part. That would've really been annoying to try to find :). I also removed the code below, as it was the main problem I was having. Do you have any examples on how to add in something to check if it contains the word? If not that's fine. Thanks for answering :).
    Code:java
    1. if (words.contains(args[1].toLowerCase())) {
    2. sender.sendMessage(args[1]
    3. + " is not a banned word!");
    4. }


    Heh, whoops. I completely just went braindead there. I fixed everything. Thanks bdubz4552 and garbagemule

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 8, 2016
  6. Offline

    garbagemule

    Just to clear up my point about the remove method: When you call remove on a list, the return value tells you whether or not the element you tried to remove was in the list or not. Thus, this snippet:
    Code:
    List<String> words = ...;
    if (words.contains(word)) {
        words.remove(word);
        // Announce successful removal
    } else {
        // Announce non-existent word
    }
    ... can be reduced to:
    Code:
    List<String> words = ...;
    if (words.remove(word)) {
        // Announce successful removal
    } else {
        // Announce non-existent word
    }
    ... i.e. you're cutting out the membership test. This approach is also faster, because it's limited to one lookup (the one necessary to locate the object in the list), whereas checking for membership first introduces an extra lookup. With tiny lists like these, though, it doesn't really matter. I just wanted to bring attention to the method signature and how it can be employed for this kind of use case :)
     
  7. Offline

    Googlelover1234

    Yeah, I read the link you told me about how it would return true if it could work. I thought about doing that, but I wasn't sure. Thanks for this! ;)
     
Thread Status:
Not open for further replies.

Share This Page