7
\$\begingroup\$

This is a (fairly) simple IRC bot. The whole idea of it was similar to Duga, except that it will post to an IRC server instead.

The bot takes messages from a SQL table and posts them in the IRC server.

It supports a few (basic) commands which are sent as private messages to the user who requested the command.

The bot is designed to pull messages from an SQL server, which means that messages are not likely to be lost. I haven't included any DDL for the SQL tables, Stored Procedures, or Views, but they all work exactly as expected.

Repository on GitHub

The Program class:

class Program
{
    private static BotConfiguration configuration;
    private static CommandDispatcher commandDispatcher;
    public static IrcClient irc = new IrcClient();

#if DEBUG
    private static string configFile = "BotConfiguration.debug.xml";
#else
    private static string configFile = "BotConfiguration.release.xml";
#endif

    static void Main(string[] args)
    {
        configuration = BotConfiguration.LoadConfig(configFile);
        _Main(args);
        Console.ReadLine();
    }

    public static void OnQueryMessage(object sender, IrcEventArgs e)
    {
        switch (e.Data.MessageArray[0])
        {
            case "dump_channel":
                string requested_channel = e.Data.MessageArray[1];
                Channel channel = irc.GetChannel(requested_channel);

                irc.SendMessage(SendType.Message, e.Data.Nick, "<channel '" + requested_channel + "'>");

                irc.SendMessage(SendType.Message, e.Data.Nick, "Name: '" + channel.Name + "'");
                irc.SendMessage(SendType.Message, e.Data.Nick, "Topic: '" + channel.Topic + "'");
                irc.SendMessage(SendType.Message, e.Data.Nick, "Mode: '" + channel.Mode + "'");
                irc.SendMessage(SendType.Message, e.Data.Nick, "Key: '" + channel.Key + "'");
                irc.SendMessage(SendType.Message, e.Data.Nick, "UserLimit: '" + channel.UserLimit + "'");

                string nickname_list = "";
                nickname_list += "Users: ";
                foreach (DictionaryEntry de in channel.Users)
                {
                    string key = (string)de.Key;
                    ChannelUser channeluser = (ChannelUser)de.Value;
                    nickname_list += "(";
                    if (channeluser.IsOp)
                    {
                        nickname_list += "@";
                    }
                    if (channeluser.IsVoice)
                    {
                        nickname_list += "+";
                    }
                    nickname_list += ")" + key + " => " + channeluser.Nick + ", ";
                }
                irc.SendMessage(SendType.Message, e.Data.Nick, nickname_list);

                irc.SendMessage(SendType.Message, e.Data.Nick, "</channel>");
                break;
            case "gc":
                GC.Collect();
                break;
            case "join":
                irc.RfcJoin(e.Data.MessageArray[1]);
                break;
            case "part":
                irc.RfcPart(e.Data.MessageArray[1]);
                break;
            case "die":
                Exit();
                break;
        }
    }

    public static void OnError(object sender, Meebey.SmartIrc4net.ErrorEventArgs e)
    {
        System.Console.WriteLine($"Error: {e.ErrorMessage}");
        Exit();
    }

    public static void OnRawMessage(object sender, IrcEventArgs e)
    {
        string message = e.Data.RawMessage;
        System.Console.WriteLine(message);

        commandDispatcher.OnMessageReceived(new MessageReceivedArgs(e.Data.RawMessage, null));
    }

    public static void _Main(string[] args)
    {
        Thread.CurrentThread.Name = "Main";

        irc.Encoding = System.Text.Encoding.UTF8;

        irc.SendDelay = 200;

        irc.ActiveChannelSyncing = true;

        irc.OnQueryMessage += new IrcEventHandler(OnQueryMessage);
        irc.OnError += new Meebey.SmartIrc4net.ErrorEventHandler(OnError);
        irc.OnRawMessage += new IrcEventHandler(OnRawMessage);

        try
        {
            irc.Connect(configuration.IrcServer, configuration.Port);
        }
        catch (ConnectionException e)
        {
            // something went wrong, the reason will be shown
            System.Console.WriteLine($"Could not connect! Reason: {e.Message}");
            Exit();
        }

#if !DEBUG
        try
#endif
        {
            irc.Login(configuration.Nickname, configuration.FullName);
            irc.RfcJoin(configuration.DefaultChannel);

            new Thread(new ThreadStart(ReadCommands)).Start();
            new Thread(new ThreadStart(ListenForAutoMessages)).Start();

            commandDispatcher = new CommandDispatcher(configuration, irc);
            commandDispatcher.OnMessageReceived(new MessageReceivedArgs(":SARA VERSION", new string[] { "force" }));

            // here we tell the IRC API to go into a receive mode, all events
            // will be triggered by _this_ thread (main thread in this case)
            // Listen() blocks by default, you can also use ListenOnce() if you
            // need that does one IRC operation and then returns, so you need then 
            // an own loop 
            irc.Listen();

            irc.Disconnect();
#if !DEBUG
        }
        catch (ConnectionException)
        {
            // this exception is handled because Disconnect() can throw a not
            // connected exception
            Console.ReadLine();
            Exit();
        }
        catch (Exception e)
        {
            System.Console.WriteLine($"Error occurred! Message: {e.Message}");
            System.Console.WriteLine($"Exception: {e.StackTrace}");
            Console.ReadLine();
            Exit();
        }
#else
        }
#endif
    }

    public static void ListenForAutoMessages()
    {
        using (SqlConnection sqlConnection = new SqlConnection(configuration.ConnectionString))
        {
            while (true)
            {
                if (sqlConnection.State != ConnectionState.Open)
                    sqlConnection.Open();

                List<Guid> sentIds = GetSentIds(sqlConnection);

                UpdateSentIds(sentIds, sqlConnection);

                Thread.Sleep(1);
            }
        }
    }

    public static List<Guid> GetSentIds(SqlConnection sqlConnection)
    {
        List<Guid> sentIds = new List<Guid>();

        using (SqlCommand sqlCommand = new SqlCommand(configuration.SqlScripts["Operations\\GetUnsentIrcMessages"], sqlConnection))
        {
            SqlDataReader reader = sqlCommand.ExecuteReader();

            if (reader.HasRows)
            {
                while (reader.Read())
                {
                    Console.WriteLine($"Message <{reader["Message"].ToString()}> with ID <{reader["Id"].ToString()}> sent at <{DateTime.UtcNow.ToString("O")}>.");
                    irc.SendMessage(SendType.Notice, configuration.DefaultChannel, reader["Message"].ToString());
                    sentIds.Add(Guid.Parse(reader["Id"].ToString()));
                }
            }

            reader.Close();
        }

        return sentIds;
    }

    public static void UpdateSentIds(List<Guid> sentIds, SqlConnection sqlConnection)
    {
        foreach (Guid guid in sentIds)
        {
            using (SqlCommand sqlCommand2 = new SqlCommand(configuration.SqlScripts["Operations\\HasSentIrcMessage"], sqlConnection))
            {
                sqlCommand2.Parameters.Add("@Guid", SqlDbType.UniqueIdentifier).Value = guid;
                sqlCommand2.ExecuteNonQuery();
            }
        }
    }

    public static void ReadCommands()
    {
        // here we read the commands from the stdin and send it to the IRC API
        // WARNING, it uses WriteLine() means you need to enter RFC commands
        // like "JOIN #test" and then "PRIVMSG #test :hello to you"
        while (true)
        {
            string cmd = System.Console.ReadLine();
            if (cmd.StartsWith("/list"))
            {
                int pos = cmd.IndexOf(" ");
                string channel = null;
                if (pos != -1)
                {
                    channel = cmd.Substring(pos + 1);
                }

                IList<ChannelInfo> channelInfos = irc.GetChannelList(channel);
                Console.WriteLine($"Channel count: {channelInfos.Count}");
                foreach (ChannelInfo channelInfo in channelInfos)
                {
                    Console.WriteLine($"Channel: {channelInfo.Channel} user count: {channelInfo.UserCount} topic: {channelInfo.Topic}");
                }
            }
            else
            {
                irc.WriteLine(cmd);
            }
        }
    }

    public static void Exit()
    {
        // we are done, lets exit...
        System.Console.WriteLine("Exiting...");
        System.Environment.Exit(0);
    }
}

The MessageReceivedEventArgs:

public class MessageReceivedArgs
{
    public string Message { get; }
    public string[] Arguments { get; }

    public MessageReceivedArgs(string message, string[] arguments)
    {
        Message = message;
        Arguments = arguments;
    }
}

The BotConfiguration:

public class BotConfiguration
{
    public string ConnectionString { get; set; }
    public string IrcServer { get;  set; }
    public string DefaultChannel { get;  set; }
    public string Nickname { get;  set; }
    public string FullName { get; set; }
    public int Port { get; set; }
    public string SqlScriptFolder { get; set; }

    [XmlIgnore]
    public Dictionary<string, string> SqlScripts { get; private set; }

    public static BotConfiguration LoadConfig(string file)
    {
        XmlSerializer xmlSerializer = new XmlSerializer(typeof(BotConfiguration));

        using (StreamReader streamReader = new StreamReader(file))
        {
            BotConfiguration config = (BotConfiguration)xmlSerializer.Deserialize(streamReader);

            List<string> files = GetAllFiles(config.SqlScriptFolder);
            config.SqlScripts = LoadSqlScripts(files, config.SqlScriptFolder);

            return config;
        }
    }

    public static Dictionary<string, string> LoadSqlScripts(List<string> files, string rootFolder)
    {
        Dictionary<string, string> scriptFiles = new Dictionary<string, string>();

        foreach (string file in files)
        {
            string tempFile = file.Substring(0, file.LastIndexOf('.'));
            scriptFiles.Add(tempFile.Replace($"{rootFolder}\\", ""), File.ReadAllText(file));
        }

        return scriptFiles;
    }

    public static List<string> GetAllFiles(string directory)
    {
        List<string> files = new List<string>();

        foreach (string directoryFile in Directory.GetFiles(directory))
        {
            files.Add(directoryFile);
        }

        foreach (string workingDirectory in Directory.GetDirectories(directory))
        {
            GetAllFiles(workingDirectory).ForEach(x => files.Add(x));
        }

        return files;
    }

    public void SaveConfig(string file)
    {
        XmlSerializer xmlSerializer = new XmlSerializer(typeof(BotConfiguration));

        using (StreamWriter streamWriter = new StreamWriter(file))
        {
            xmlSerializer.Serialize(streamWriter, this);
        }
    }
}

The CommandDispatcher:

public class CommandDispatcher
{
    public BotConfiguration Configuration { get; }
    public IrcClient IrcClient { get; }

    private delegate void _dispatcher(string message, string user, params string[] args);
    private Dictionary<string, _dispatcher> _delegateMap = new Dictionary<string, _dispatcher>();

    public CommandDispatcher(BotConfiguration configuration, IrcClient irc)
    {
        Configuration = configuration;
        IrcClient = irc;

        _delegateMap.Add("VERSION", VersionMethod);
        _delegateMap.Add("COUNT", CountMethod);
        _delegateMap.Add("DATE", DateMethod);
        _delegateMap.Add("AUTOMESSAGE", AutoMessageMethod);
    }

    private void AutoMessageMethod(string message, string user, string[] args)
    {
        IMessageHandler handler = new AutoMessageHandler();

        handler.ProcessMethod(IrcClient, Configuration, message, user, args);
    }

    private void VersionMethod(string message, string user, string[] args)
    {
        IMessageHandler handler = new VersionHandler();

        handler.ProcessMethod(IrcClient, Configuration, message, user, args);
    }

    private void DateMethod(string message, string user, string[] args)
    {
        IMessageHandler handler = new DateHandler();

        handler.ProcessMethod(IrcClient, Configuration, message, user, args);
    }

    private void CountMethod(string message, string user, string[] args)
    {
        IMessageHandler handler = new CountHandler();

        handler.ProcessMethod(IrcClient, Configuration, message, user, args);
    }

    public void OnMessageReceived(MessageReceivedArgs e)
    {
        ProcessMessage(e);
    }

    private void ProcessMessage(MessageReceivedArgs e)
    {
        string commandPortion = e.Message.Substring(e.Message.ToUpper().LastIndexOf(":SARA") + 1);
        string[] messagePortions = commandPortion.Split(new char[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);

        if (commandPortion.ToUpper().IndexOf($"{Configuration.Nickname.ToUpper()}") == 0 && messagePortions.Length >= 2)
        {
            string user = "";

            if (e.Message.IndexOf(':') == 0 && e.Message.IndexOf('!') > 1)
                user = e.Message.Substring(1, e.Message.IndexOf('!') - 1);

            string[] args = e.Arguments;
            int offset = 2;

            if (e.Arguments != null)
            {
                offset += e.Arguments.Length;
                for (int i = 0; i < e.Arguments.Length; i++)
                {
                    args[i] = e.Arguments[i];
                }
            }

            if (args == null)
            {
                args = new string[messagePortions.Length - 2 + offset - 2];
            }

            for (int i = offset; i < messagePortions.Length; i++)
            {
                args[i - offset] = messagePortions[i];
            }

            if (_delegateMap.ContainsKey(messagePortions[1].ToUpper()))
            {
                _delegateMap[messagePortions[1].ToUpper()].Invoke(commandPortion, user, args);
            }
        }
    }
}

An IMessageHandler:

public interface IMessageHandler
{
    void ProcessMethod(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args);
}

Then the Message Handlers, starting with AutoMessageHandler:

public class AutoMessageHandler : IMessageHandler
{
    public void ProcessMethod(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
    {
        if (args.Length >= 1 && args[0].ToUpper() == "LATEST")
        {
            using (SqlConnection connection = new SqlConnection(configuration.ConnectionString))
            {
                connection.Open();
                using (SqlCommand command = new SqlCommand(configuration.SqlScripts["Operations\\GetLastAutoMessages"], connection))
                {
                    command.Parameters.Add("@Number", SqlDbType.Int).Value = 1;

                    SendMessage(command, ircClient, configuration, message, user, args);
                }
            }
        }
        else if (args.Length >= 1 && args[0].ToUpper() == "TODAY")
        {
            using (SqlConnection connection = new SqlConnection(configuration.ConnectionString))
            {
                connection.Open();
                using (SqlCommand command = new SqlCommand(configuration.SqlScripts["Operations\\GetAllIrcMessages"], connection))
                {
                    command.Parameters.Add("@TimeLimitValue", SqlDbType.DateTime2).Value = DateTime.Today;

                    SendMessage(command, ircClient, configuration, message, user, args);
                }
            }
        }
        else
        {
            SendMessage(ircClient, configuration, $"Please specify either TODAY or LATEST after the command.", user, args);
        }
    }

    private void SendMessage(SqlCommand command, IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
    {
        SqlDataReader reader = command.ExecuteReader();
        if (reader.HasRows)
        {
            while (reader.Read())
            {
                Console.WriteLine($"Message <{reader["Message"].ToString()}> with ID <{reader["Id"].ToString()}> sent at <{DateTime.UtcNow.ToString("O")}>.");

                SendMessage(ircClient, configuration, reader["Message"].ToString(), user, args);
            }
        }
    }

    private void SendMessage(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
    {
        if (user != "")
        {
            ircClient.RfcPrivmsg($"{user}", message.ToString());
        }
        else
        {
            ircClient.SendMessage(SendType.Notice, configuration.DefaultChannel, message.ToString());
        }
    }
}

We have a CountMessageHandler:

public class CountMessageHandler : IMessageHandler
{
    public void ProcessMethod(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
    {
        using (SqlConnection connection = new SqlConnection(configuration.ConnectionString))
        {
            connection.Open();

            if (args.Length >= 1 && args[0].ToUpper() == "IRC")
            {
                if (args.Length >= 2 && args[1].ToUpper() == "TOTAL")
                {
                    using (SqlCommand command = new SqlCommand(configuration.SqlScripts["Operations\\GetAllIrcMessagesCount"], connection))
                    {
                        //ircClient.SendMessage(SendType.Notice, configuration.DefaultChannel, $"{messagePrefix}Total messages: {command.ExecuteScalar()}");
                        ircClient.RfcPrivmsg($"{user}", $"Total messages: {command.ExecuteScalar()}");
                    }
                }
                else if (args.Length >= 2 && args[1].ToUpper() == "TODAY")
                {
                    using (SqlCommand command = new SqlCommand(configuration.SqlScripts["Operations\\GetIrcMessagesTodayCount"], connection))
                    {
                        //ircClient.SendMessage(SendType.Notice, configuration.DefaultChannel, $"{messagePrefix}Total messages today: {command.ExecuteScalar()}");
                        ircClient.RfcPrivmsg($"{user}", $"Total messages today: {command.ExecuteScalar()}");
                    }
                }
            }
        }
    }
}

A DateMessageHandler:

public class DateMessageHandler : IMessageHandler
{
    private string _defaultDateTime => "O";

    public void ProcessMethod(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
    {
        if (args.Length >= 1)
        {
            if (args[0].ToUpper() == "UTC")
            {
                SendMessage(new DateTimeOffset(DateTime.UtcNow), ircClient, configuration, message, user, args);
            }
            else if (args[0].ToUpper() == "LOCAL")
            {
                SendMessage(new DateTimeOffset(DateTime.Now), ircClient, configuration, message, user, args);
            }
        }
        else
        {
            SendMessage(new DateTimeOffset(DateTime.UtcNow), ircClient, configuration, message, user, args);
        }
    }

    private void SendMessage(DateTimeOffset time, IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
    {
        try
        {
            if (args.Length >= 2)
            {
                string format = GetFormat(args);
                ircClient.RfcPrivmsg($"{user}", $"Date/time: {time.ToString(format)}");
            }
            else
            {
                ircClient.RfcPrivmsg($"{user}", $"Date/time: {time.ToString(_defaultDateTime)}");
            }
        }
        catch
        {
            ircClient.RfcPrivmsg($"{user}", $"Invalid date/time format presented.");
        }
    }

    private string GetFormat(params string[] args)
    {
        string format = "";

        for (int i = 1; i < args.Length; i++)
        {
            if (format.Length > 0)
            {
                format += " ";
            }

            format += args[i];
        }

        return format;
    }
}

A VersionMessageHandler:

public class VersionMessageHandler : IMessageHandler
{
#if DEBUG
    public string ProgramMode = "Debug";
#else
    public string ProgramMode = "Release";
#endif

    public string Version { get; }

    public VersionMessageHandler()
    { 
        Version = typeof(Program).Assembly.GetName().Version.ToString();
    }

    public void ProcessMethod(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
    {
        if (user != "")
        {
            ircClient.RfcPrivmsg($"{user}", $"Sara Version: {Version} {ProgramMode}");
        }

        if (args.Length > 0 && args[0] == "force")
        {
            ircClient.SendMessage(SendType.Notice, configuration.DefaultChannel, $"Sara Version: {Version} {ProgramMode}");
        }
    }
}

I'm mostly interested in anything I can abstract out further, but as usual, any comments are welcome.

\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

Lots of code we got. This from bottom to top we review.

VersionMessageHandler

#if DEBUG
    public string ProgramMode = "Debug";
#else
    public string ProgramMode = "Release";
#endif  

Why is this field public accessible ? Any caller/user of this class can adjust it to its needs. If this is set to null by its caller, the call to ProcessMethod() will lead to unexpected results.

public VersionMessageHandler()
{ 
    Version = typeof(Program).Assembly.GetName().Version.ToString();
}  

this will couple the VersionMessageHandler class tigthly to the Program class. It would be better to pass the version as a constructor argument.

public void ProcessMethod(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
{
    if (user != "")
    {
        ircClient.RfcPrivmsg($"{user}", $"Sara Version: {Version} {ProgramMode}");
    }

    if (args.Length > 0 && args[0] == "force")
    {
        ircClient.SendMessage(SendType.Notice, configuration.DefaultChannel, $"Sara Version: {Version} {ProgramMode}");
    }
}  

The check for if (user != "") is superflous because you don't check for user != null. Using string interpolation with a argument which is null behaves the same as it wouldn't be present. So $"{user}" would evaluate for user == null or user == "" to just "".

Now you know what I meant by unexpected results for setting ProgramMode = null. The second if would send Sara Version: {Version} where Version would be replaced by the current version, but the ProgramMode would be skipped.


DateMessageHandler

Using expression bodied member for assigning a value to a string like private string _defaultDateTime => "O"; is a little bit overkill. In addition because it won't be changed anywhere in the code, you should just make it private readonly.

public void ProcessMethod(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
{
    if (args.Length >= 1)
    {
        if (args[0].ToUpper() == "UTC")
        {
            SendMessage(new DateTimeOffset(DateTime.UtcNow), ircClient, configuration, message, user, args);
        }
        else if (args[0].ToUpper() == "LOCAL")
        {
            SendMessage(new DateTimeOffset(DateTime.Now), ircClient, configuration, message, user, args);
        }
    }
    else
    {
        SendMessage(new DateTimeOffset(DateTime.UtcNow), ircClient, configuration, message, user, args);
    }
}  

here you have small code duplication wich can be removed by changing the method like so

public void ProcessMethod(IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
{
    if (args.Length == 0 || args[0].ToUpper() == "UTC")
    {
        SendMessage(new DateTimeOffset(DateTime.UtcNow), ircClient, configuration, message, user, args);
    }
    else if (args[0].ToUpper() == "LOCAL")
    {
        SendMessage(new DateTimeOffset(DateTime.Now), ircClient, configuration, message, user, args);
    }
}

For using string.ToUpper() for comparising please read here: using-invariantcultureignorecase-instead-of-toupper-for-case-insensitive-string

Quoting the qoute:

As discussed by lots and lots of people, the "I" in Turkish behaves differently than in most languages. Per the Unicode standard, our lowercase "i" becomes "İ" (U+0130 "Latin Capital Letter I With Dot Above") when it moves to uppercase. Similarly, our uppercase "I" becomes "ı" (U+0131 "Latin Small Letter Dotless I") when it moves to lowercase.

Fix: Again, use an ordinal (raw byte) comparer, or invariant culture for comparisons unless you absolutely need culturally based linguistic comparisons (which give you uppercase I's with dots in Turkey)

private string GetFormat(params string[] args)
{
    string format = "";

    for (int i = 1; i < args.Length; i++)
    {
        if (format.Length > 0)
        {
            format += " ";
        }

        format += args[i];
    }

    return format;
}  

this method can be reduced to a one liner and therfor could just be removed completely.

Just replace the call of the method like so

string format = string.Join(" ", args);  

CountMessageHandler

This if construct

       if (args.Length >= 1 && args[0].ToUpper() == "IRC")
       {
           if (args.Length >= 2 && args[1].ToUpper() == "TOTAL")
           {
               using (SqlCommand command = new SqlCommand(configuration.SqlScripts["Operations\\GetAllIrcMessagesCount"], connection))
               {
                   //ircClient.SendMessage(SendType.Notice, configuration.DefaultChannel, $"{messagePrefix}Total messages: {command.ExecuteScalar()}");
                   ircClient.RfcPrivmsg($"{user}", $"Total messages: {command.ExecuteScalar()}");
               }
           }
           else if (args.Length >= 2 && args[1].ToUpper() == "TODAY")
           {
               using (SqlCommand command = new SqlCommand(configuration.SqlScripts["Operations\\GetIrcMessagesTodayCount"], connection))
               {
                   //ircClient.SendMessage(SendType.Notice, configuration.DefaultChannel, $"{messagePrefix}Total messages today: {command.ExecuteScalar()}");
                   ircClient.RfcPrivmsg($"{user}", $"Total messages today: {command.ExecuteScalar()}");
               }
           }
       }

should be cleaned (remove commented out code) and simplified like so

        if (args.Length >= 2 && args[0].ToUpper() == "IRC")
        {
            if (args[1].ToUpper() == "TOTAL")
            {
                using (SqlCommand command = new SqlCommand(configuration.SqlScripts["Operations\\GetAllIrcMessagesCount"], connection))
                {
                    ircClient.RfcPrivmsg($"{user}", $"Total messages: {command.ExecuteScalar()}");
                }
            }
            else if (args[1].ToUpper() == "TODAY")
            {
                using (SqlCommand command = new SqlCommand(configuration.SqlScripts["Operations\\GetIrcMessagesTodayCount"], connection))
                {
                    ircClient.RfcPrivmsg($"{user}", $"Total messages today: {command.ExecuteScalar()}");
                }
            }
        }  

changing the ToUpper() would be needed here too.


AutoMessageHandler

I won't mention the ToUpper() problem from now on, I guess you won't need any more remainders ;-)

Why do you always check for args.Length >= 1 instead of args.Length > 0 ?

private void SendMessage(SqlCommand command, IrcClient ircClient, BotConfiguration configuration, string message, string user, params string[] args)
{
    SqlDataReader reader = command.ExecuteReader();
    if (reader.HasRows)
    {
        while (reader.Read())
        {
            Console.WriteLine($"Message <{reader["Message"].ToString()}> with ID <{reader["Id"].ToString()}> sent at <{DateTime.UtcNow.ToString("O")}>.");

            SendMessage(ircClient, configuration, reader["Message"].ToString(), user, args);
        }
    }
}   

You should enclose the SqlDataReader in a using statement to dispose it properly.


CommandDispatcher

I had been so pleased to see you using braces for single line if statements until I saw the ProcessMessage() method. There is this small

        if (e.Message.IndexOf(':') == 0 && e.Message.IndexOf('!') > 1)
            user = e.Message.Substring(1, e.Message.IndexOf('!') - 1);

I don't get what you are doing here

        string[] args = e.Arguments;
        int offset = 2;

        if (e.Arguments != null)
        {
            offset += e.Arguments.Length;
            for (int i = 0; i < e.Arguments.Length; i++)
            {
                args[i] = e.Arguments[i];
            }
        }

the only part needed is the offset += e.Arguments.Length; if e.Arguments != null. Because you assigned e.Arguments to string[] args it would be more obvious to use args instead of e.Arguments like so

        string[] args = e.Arguments;
        int offset = 2;

        if (args != null)
        {
            offset += args.Length;
        }

The next part I don't really understand either

        if (args == null)
        {
            args = new string[messagePortions.Length - 2 + offset - 2];
        }

args can only be null if e.Arguments is null and then offset will be 2. So a simple else to the part above would be sufficient and + offset -2could just be omitted like so

        string[] args = e.Arguments;
        int offset = 2;

        if (args != null)
        {
            offset += args.Length;
        }
        else
        {
            args = new string[messagePortions.Length - 2];
        }

which leads us now to this

        for (int i = offset; i < messagePortions.Length; i++)
        {
            args[i - offset] = messagePortions[i];
        }  

I don't know how big this messagePortions array can get and how many Arguments can be passed by the event, but let us assume we have 2 arguments and messagePortions contains 3 items.

This loop will then try to stuff 3 items in 2 slots which will lead to an IndexOutOfRangeException.

So let us consider another szenario where e.Arguments had been null and messagePortions contains 3 items. In this case the args variable will be initialized to

args = new string[messagePortions.Length - 2];  

aka

args = new string[3 - 2];  

which would lead to the same IndexOutOfRangeException.

I guess you should dig into this some more.

Using ContainsKey() together with the getter of the Item property of a Dictionary<TKey, TValue> should be replaced by a call to TryGetValue() which is faster because the check if the key exists is done by the Item getter too.

So this

       if (_delegateMap.ContainsKey(messagePortions[1].ToUpper()))
       {
           _delegateMap[messagePortions[1].ToUpper()].Invoke(commandPortion, user, args);
       }

should become this

        _dispatcher current; 

        if (_delegateMap.TryGetValue(messagePortions[1].ToUpper(), out current))
        {
            current.Invoke(commandPortion, user, args);
        }  

which in addition is much more readable.


BotConfiguration

public Dictionary<string, string> SqlScripts { get; private set; }  

Do you know that a caller can adjust/replace the content of the dictionary items ? Having a private setter only prevents the whole dictionary to be changed/replaced but the values can nevertheless be changed.

A more secure way would be to use a ReadOnlyDictionary<TKey, TValue>.

public static List<string> GetAllFiles(string directory)
{
    List<string> files = new List<string>();

    foreach (string directoryFile in Directory.GetFiles(directory))
    {
        files.Add(directoryFile);
    }

    foreach (string workingDirectory in Directory.GetDirectories(directory))
    {
        GetAllFiles(workingDirectory).ForEach(x => files.Add(x));
    }

    return files;
}

Why is this method public ? It seems there is no reason for that.

By using the right tool for the job this should be simplified by using the AddRange() method of the List<T> and the overloaded constructor of the List<T> which takes an IEnumerable<T> (if you don't want to return an IList<T>) like so

private static List<string> GetAllFiles(string directory)
{
    List<string> files = new List<string>(Directory.GetFiles(directory));

    foreach (string workingDirectory in Directory.GetDirectories(directory))
    {
        files.AddRange(GetAllFiles(workingDirectory));
    }

    return files;
}

The LoadSqlScripts() doesn't need to be public either. And the first parameter would be better an IEnumerable<string> because you just iterate over the items and don't use any method of the List<T>.


Program

public static IrcClient irc = new IrcClient();

Again, why is this public ?

OnQueryMessage()

requested_channel snake casing, really ????

irc.SendMessage(SendType.Message, e.Data.Nick, "<channel '" + requested_channel + "'>");

irc.SendMessage(SendType.Message, e.Data.Nick, "Name: '" + channel.Name + "'");
irc.SendMessage(SendType.Message, e.Data.Nick, "Topic: '" + channel.Topic + "'");
irc.SendMessage(SendType.Message, e.Data.Nick, "Mode: '" + channel.Mode + "'");
irc.SendMessage(SendType.Message, e.Data.Nick, "Key: '" + channel.Key + "'");
irc.SendMessage(SendType.Message, e.Data.Nick, "UserLimit: '" + channel.UserLimit + "'");

why don't you use string interpolation like you did in the rest of the classes ?

    string nickname_list = "";
    nickname_list += "Users: ";
    foreach (DictionaryEntry de in channel.Users)
    {
        string key = (string)de.Key;
        ChannelUser channeluser = (ChannelUser)de.Value;
        nickname_list += "(";
        if (channeluser.IsOp)
        {
            nickname_list += "@";
        }
        if (channeluser.IsVoice)
        {
            nickname_list += "+";
        }
        nickname_list += ")" + key + " => " + channeluser.Nick + ", ";
    }  

this ^^^^^^ screams for using a StringBuilder.

public static void OnRawMessage(object sender, IrcEventArgs e)
{
    string message = e.Data.RawMessage;
    System.Console.WriteLine(message);

    commandDispatcher.OnMessageReceived(new MessageReceivedArgs(e.Data.RawMessage, null));
}

the message variable is only used once and should be omitted like so

public static void OnRawMessage(object sender, IrcEventArgs e)
{
    System.Console.WriteLine(e.Data.RawMessage);

    commandDispatcher.OnMessageReceived(new MessageReceivedArgs(e.Data.RawMessage, null));
}

In the GetSentIds() method the SqlDataReader should be enclosed in an using statement.

public static void UpdateSentIds(List<Guid> sentIds, SqlConnection sqlConnection)
{
    foreach (Guid guid in sentIds)
    {
        using (SqlCommand sqlCommand2 = new SqlCommand(configuration.SqlScripts["Operations\\HasSentIrcMessage"], sqlConnection))
        {
            sqlCommand2.Parameters.Add("@Guid", SqlDbType.UniqueIdentifier).Value = guid;
            sqlCommand2.ExecuteNonQuery();
       }
    }
}

Why is the SqlCommend variable named sqlCommand2 ? I couldn't find a sqlCommand1 there ;-)

This method can be improved by placing the using (SqlCommand... outside of the loop and creating the parameter once like so

public static void UpdateSentIds(List<Guid> sentIds, SqlConnection sqlConnection)
{
    using (SqlCommand sqlCommand = new SqlCommand(configuration.SqlScripts["Operations\\HasSentIrcMessage"], sqlConnection))
    {
        SqlParameter parameter = sqlCommand.Parameters.Add("@Guid", SqlDbType.UniqueIdentifier);
        foreach (Guid guid in sentIds)
        {
            parameter.Value = guid;
            sqlCommand.ExecuteNonQuery();
        }
    }
}
public static void ReadCommands()
{
    // here we read the commands from the stdin and send it to the IRC API
    // WARNING, it uses WriteLine() means you need to enter RFC commands
    // like "JOIN #test" and then "PRIVMSG #test :hello to you"
    while (true)
    {
        string cmd = System.Console.ReadLine();
        if (cmd.StartsWith("/list"))
        {
            int pos = cmd.IndexOf(" ");
            string channel = null;
            if (pos != -1)
            {
                channel = cmd.Substring(pos + 1);
            }

            IList<ChannelInfo> channelInfos = irc.GetChannelList(channel);
            Console.WriteLine($"Channel count: {channelInfos.Count}");
            foreach (ChannelInfo channelInfo in channelInfos)
            {
                Console.WriteLine($"Channel: {channelInfo.Channel} user count: {channelInfo.UserCount} topic: {channelInfo.Topic}");
            }
        }
        else
        {
            irc.WriteLine(cmd);
        }
    }
}  

If one types only /list the call to irc.GetChannelList() will blow with an ArgumentNullException.

Please recheck all public methods, properties and variables if they really need to be public.

Reviewed the code now we have.

\$\endgroup\$
1
  • \$\begingroup\$ I had actually done a few of these after I posted the question, but there were still a lot I hadn't taken care of. Thanks greatly for the review! \$\endgroup\$ Sep 23, 2015 at 14:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.