Music & DJ IRC bot

I wrote an IRC bot in Java using Pircbot for use in twitch(.tv) chat. It is intended to be run locally and to only be connected to one channel at a time. When it is connected to my channel, it will take song requests for MP3s in my music directory. There is a songlist on a Google site. Otherwise I'll just use it in a friend's channel for random fun. Right now it works. I'm interested in:

Is there a better way to handle the main logic? Being a bot, logic is controlled by strings it receives. Right now I check if something triggers a response, if not I move on down the line. The conditions and the responses aren't set in stone and when I use it, I normally run in debug mode so I can hotswap features.

Is my code organization appropriate? Everything could be in onMessage. I tried to keep indenting low and purpose clear. Still with a bunch of random unrelated commands it is hard to organize.

How is the documentation? I don't feel like a lot of it is necessary but I was just messing around. I would like my code to be clean enough so that anyone on twitch could copy my example but I honestly don't expect anyone to ever use this code.

Anything else worth noting?

The bot:

```
package bot;

import java.net.MalformedURLException;
import java.net.URL;

import mp3.Player;

import org.jibble.pircbot.PircBot;

public class Linksbutt extends PircBot {

private final String fSpace = "      ";
private final String dir = "file:///C:/Users/fredflintstoneviii/Music/";
private final String sr = "!sr";
private final String sngreq = "!songrequest";
private final String myChannel = "#linkviii";
private boolean srOn;
private Player player;

public Linksbutt() {
this.setName("linksbutt");
this.setMessageDelay(2000);
player = new Player();
srOn = true;
}

/**
* handels recived messages
*/
@Override
public void onMessage(String channel, String sender, String login,
String hostname, Stri

Some generals advices

If your making documentation for your code, at least put a bit of effort in it. Incomplete or badly written helps nobody.

/**
 * handels recived messages
 */

I don't know if you're a native speaker or not (I'm not), but for 3 words, 2 are not written correctly. It should be handles received messages. The other problem is this does not give much explanation about what your method is doing. I'm not a fan of documentation, but if you don't use it, remove it because it's just noise at the moment.

Suppressed warnings

I should not see @SuppressWarnings("unused") in code without some explanations of why it's necessary. Those warnings should not be suppress without a good reason because most of the time they are code smells.

Constant

You mention that this program could be used by other peoples in some point in the future (Not really but it could be a possibility that you mentionned). Well I would not expect the majority of Twitch user to be programmer (even hobbyist) in Java. You have a some constants referring to "personal" information :

private final String dir = "file:///C:/Users/fredflintstoneviii/Music/";
private final static String dir = "file:///C:/Users/fredflintstoneviii/Music/";
private final static String nul = "file:///C:/Users/fredflintstoneviii/Desktop/Pi/Other/Other%20media/audio/sound/null.mp3";

If I want to use your program, I will need to open the source, edit those values and then package everything up and finally use it. For that kind of parameters, you could have a special file to configure your application. A properties file is pretty common (there is a class that can load a file like this automaticly), but there is a ton of formats you could adopt.

FrankerZ

Well if you can count a String more than 3 times directly written in a source, well it should be a constant.

Style convention

You're not always following the Java convention. I would recommend following them since they will make your code easier to read for other Java developer. You can also look at the Google Java style.

I prefer to always use {}, since it prevents some weird bugs to pop up from nowhere when you will change your code and forget to add them. This is more of a personal taste, so take it or leave it.
Code

if (channel.equalsIgnoreCase(myChannel)
            && linkviiiCmd(channel, sender, message)) {
        ;
    }

You should not have empty if block in your code. It's just counter-intuitive. You're specifying a special treatment in your method, but do nothing. You could add a return statement, since this look like what you want.

The other things that I find a bit odd, is that linkviiCmd() is actually doing a treatment important. It's not just some conditions. It's not necessarily a bad thing, but I would like it to be more clear. You could separate this if from the other one like.

if (channel.equalsIgnoreCase(myChannel) {
            if (!linkviiiCmd(channel, sender, message)) {
                return;
            }
    }

The next else if would simply be an if. This change a bit your treatment and your logic, but I think it would be beneficial.

Scalability

At the moment, your code does not quite scale easily. If you need to add a command it will be an additional if case in your onMessage. You could have somethig a bit more easier to implement a new command. I'm thinking with a class that would have a method like validate or apply that would check if the command is the right one. You could then could then call execute or something similar to actually do the treatment.

This would help encapsulate the command principle in your program, and maybe reduce that big if block in your method.

StackExchange Code Review Q#58496, answer score: 7