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.
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.
1 Answer 1
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 -2
could 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.
-
\$\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\$Der Kommissar– Der Kommissar2015年09月23日 14:09:41 +00:00Commented Sep 23, 2015 at 14:09