15
\$\begingroup\$

I'd say currently my understanding of C# is within the range of (1Beginner-10Intermediate) a solid 4-5. Thus I am seeking to broaden my understanding of C#; I've started coding an IRC bot to improve. This is the code:

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Sockets;
using System.Text;
using System.Text.RegularExpressions;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

namespace IRCBot
{

User and Message classes for JSON deserialization

    internal class User
    {
        public User()
        {
            Messages = new List<Message>();
        }

        public string Name { get; set; }
        public int Access { get; set; }
        public DateTime Seen { get; set; }
        public List<Message> Messages { get; set; }
    }

    internal class Message
    {
        public string Sender { get; set; }
        public string Contents { get; set; }
        public DateTime Date { get; set; }
    }

Config struct

    internal struct Config
    {
        public bool Joined;
        public string Server;
        public string[] Channels;
        public string Nick;
        public string Name;
        public int Port;
    }

IRCBot class, where everything happens

internal class IRCBot : IDisposable, IModule {
        private readonly Dictionary<string, List<string>> users = new Dictionary<string, List<string>>();
        private readonly Dictionary<string, int> userAttempts = new Dictionary<string, int>();

        private readonly Dictionary<string, string> commands = new Dictionary<string, string>
        {
            {"join", "(join <channel>) - joins specified channel."},
            {"part", "(part <channel> <message*>) - parts specified channel."},
            {"leave", "(part <channel> <message*>) - parts specified channel."},
            {"say", "(say <channel*> <message>) - sends privmsg to target channel."},
            {"chanlist", "Provides a list of channels joined."},
            {"channels", "Provides a list of channels joined."},
            {"userlist", "List all users currently in database and their access number."},
            {"define", "(define <word> <part of speech*>) - outputs a dictionary definition for the specified word."},
            {"lookup", "(lookup <target article>) - outputs the contents of a specified wikipedia article."},
            {"message", "(message <recipient> <mesaage>) - sends a message to specified user, to be sent when they login again."},
            {"seen", "(seen <target>) - Outputs the last DateTime of the target user being active."},
            {"about", "Print out the version and creator information of the bot."},
            {"shutdown", "Closes program."}
        };

        private Config _config;
        private TcpClient _connection;
        private StreamWriter _log;
        private NetworkStream _ns;
        private StreamReader _sr;
        private StreamWriter _sw;

        private readonly List<string> channels = new List<string>();
        private readonly List<string> nameDenyList = new List<string>()
        {
            "NickServ", "ChanServ",
            "vervet.foonetic.net",
            "Eve"
        };
        private List<User> userList;

        private string recipient;
        // set config 
        public IRCBot(Config config) {
            _config = config;
        }

        public void Dispose() {
            _sr?.Close();
            _sw?.Close();
            _ns?.Close();
            _log?.Close();
            _connection?.Close();
        }

        public void OnChannelEvent(ChannelEvent e) {
            throw new NotImplementedException();
        }

        public void OnChannelMessage(ChannelMessage c) {
            recipient = c.Recipient;

            if (QueryName(c.Realname) == null
                && !nameDenyList.Contains(c.Realname)) {
                userList.Add(new User {
                    Name = c.Realname,
                    Access = 2,
                    Seen = DateTime.UtcNow
                });
            } else if (QueryName(c.Realname) != null)
                userList.First(e => e.Name == c.Realname).Seen = DateTime.UtcNow;

            if (!userAttempts.ContainsKey(c.Realname))
                userAttempts.Add(c.Realname, 0);
            if (!users.ContainsKey(c.Recipient)
                && c.Recipient.StartsWith("#"))
                users.Add(c.Recipient, new List<String>());

            File.WriteAllText("users.json", JsonConvert.SerializeObject(userList, Formatting.Indented));

            string[] arg = null;

            if (c.Args != null) {
                arg = c.Args.ToLower().Split(new char[] { ' ' }, 4);
            }

            switch (c.Type) {
                case "PRIVMSG":
                    if (arg == null) return;
                    if (QueryName(c.Realname) == null) return;

                    var access = userList.First(e => e.Name == c.Realname).Access;
                    string msg,
                        chan;

                    if (arg[0] != "eve"
                        && arg[0] != "eve,")
                        return;

                    if (UserTimeout(c.Realname)) return;

                    if (arg[1] != null)
                        switch (arg[1]) {
                            case "join":
                                if (access > 1) {
                                    SendMessage("Insufficient permissions.");
                                    return;
                                }

                                if (String.IsNullOrEmpty(arg[2])) {
                                    SendMessage("Insufficient parameters. Type 'eve help join' to view correct usage.");
                                }

                                if (!arg[2].StartsWith("#")) {
                                    SendMessage("Channel argument must be a proper channel name (i.e. starts with '#').");
                                    return;
                                }

                                if (channels.Contains(arg[2]))
                                {
                                    SendMessage("I'm already in that channel.");
                                    return;
                                }

                                Join(arg[2]);
                                break;
                            case "part":
                            case "leave":
                                if (access > 1) {
                                    SendMessage("Insufficient permissions.");
                                    return;
                                }

                                if (String.IsNullOrEmpty(arg[2])) {
                                    SendMessage("Insufficient parameters. Type 'eve help part' to view correct usage.");
                                    return;
                                }

                                if (!arg[2].StartsWith("#")) {
                                    SendMessage("Channel argument must be a proper channel name (i.e. starts with '#').");
                                    return;
                                }

                                if (!channels.Contains(arg[2])) {
                                    SendMessage("I'm not in that channel.");
                                    return;
                                }

                                Part(arg[2], arg[3]);
                                break;
                            case "say":
                                if (arg.Length < 3
                                    || (arg[2].StartsWith("#")
                                        && arg.Length < 4)) {
                                    SendMessage("Insufficient parameters. Type 'eve help say' to view correct usage.");
                                    return;
                                }

                                msg = (!arg[2].StartsWith("#") && arg.Length > 4)
                                    ? arg[2] + " " + arg[3]
                                    : arg[2];
                                chan = (arg[2].StartsWith("#")) ? arg[2] : null;

                                Say(chan, msg);
                                break;
                            case "chanlist":
                            case "channels":
                                SendMessage(string.Join(" ", channels.ToArray()));
                                break;
                            case "define":
                                if (arg.Length < 3) {
                                    SendMessage("Insufficient parameters. Type 'eve help define' to view correct usage.");
                                    return;
                                }

                                var pos = (arg.Length < 4) ? null : arg[3];

                                Define(arg[2], pos);
                                break;
                            case "lookup":
                                if (arg.Length < 3) {
                                    SendMessage("Insufficient parameters. Type 'eve help lookup' to view correct usage.");
                                    return;
                                }

                                var query = (arg.Length < 4) ? arg[2] : arg[2] + "%20" + arg[3];
                                query = query.Replace(" ", "%20");

                                Lookup(query, c.Nickname);
                                break;
                            case "message":
                                if (arg.Length < 3
                                    || arg.Length < 4) {
                                    SendMessage("Insufficient parameters. Type 'eve help message' to view correct usage.");
                                    return;
                                }

                                if (QueryName(arg[2]) == null) {
                                    SendMessage("User does not exist in database.");
                                    return;
                                }

                                Message(c.Realname, arg[2], Regex.Escape(arg[3]));
                                break;
                            case "help":
                                if (arg.Length > 3
                                    && !commands.ContainsKey(arg[2])) {
                                    SendMessage("Command does not exist.");
                                    return;
                                }

                                var cmd = (arg.Length < 3) ? null : arg[2];

                                Help(c.Nickname, cmd);
                                break;
                            case "userlist":
                                Userlist();
                                break;
                            case "seen":
                                if (arg.Length < 3) {
                                    SendMessage("Insufficient parameters. Type 'eve help message' to view correct usage.");
                                    return;
                                }

                                if (QueryName(arg[2]) == null) {
                                    SendMessage("User does not exist in database.");
                                    return;
                                }

                                Seen(arg[2]);
                                break;
                            case "about":
                                SendMessage("Evealyn Bot is a sophisticated and damn fine IRC bot created by SemiViral. Version 1.0");
                                break;
                            case "shutdown":
                                if (access > 1) {
                                    SendMessage("Goodybe.");
                                    Eve.Run = false;
                                } else SendMessage("Insufficient permissions.");
                                break;
                            default:
                                SendMessage("Invalid command.");
                                break;
                        } else
                        SendMessage("Please provide a command. Type 'eve help' to obtain my command list.");

                    break;
                case "JOIN":
            Console.WriteLine(c.Realname);
                    if (c.Realname == "Eve") return;

                    if (QueryName(c.Realname) != null) {
                        foreach (var m in userList.First(e => e.Name == c.Realname).Messages)
                            SendData("PRIVMSG", c.Nickname + " (" + m.Date + ") " + m.Sender + ": " + Regex.Unescape(m.Contents));

                        userList.First(e => e.Name == c.Realname).Messages = null;
                    }

                    users[c.Recipient].Add(c.Realname);
                    break;
                case "PART":
                    users[c.Recipient].Remove(c.Realname);
                    break;
                case "MODE":
                    if (!_config.Joined)
                    {
                        _config.Joined = true;
                        SendData("PRIVMSG", "NICKSERV IDENTIFY evepass");

                        foreach (var s in _config.Channels)
                        {
                            Join(s);
                        }
                    }
                    break;
                case "353":
                    var key = Regex.Matches(c.Args, @"(#\w+)");
                    var list = key.Cast<Match>().Select(match => match.Value).ToList();

                    foreach (var s in c.Args.Split(':')[1].Split(' '))
                        users[list[0]].Add(s);
                    break;
            }

            File.WriteAllText("users.json", JsonConvert.SerializeObject(userList, Formatting.Indented));
        }



        // command operations for main switch()
        private void Join(string chan)
        {
            SendMessage("Joining " + chan + ".");
            SendData("JOIN", chan);
            channels.Add(chan);
        }

        private void Part(string chan, string msg) {
            if (msg == null) {
                SendMessage("Leaving channel " + chan + ".");
                SendData("PART", chan);
            } else {
                SendMessage("Leaving channel " + chan + " for reason: " + msg);
                SendData("PART", chan + " " + msg);
            }

            channels.Remove(chan);
            users.Remove(chan);
        }

        private void Say(string chan, string msg) {
            if (String.IsNullOrEmpty(chan))
                SendMessage(msg);
            else
                SendData("PRIVMSG", chan + " " + msg);
        }

        private void Define(string word, string pos) {
            var url = (String.IsNullOrEmpty(pos))
                ? "https://api.pearson.com:443/v2/dictionaries/lasde/entries?headword=" + word + "&limit=1"
                : "https://api.pearson.com:443/v2/dictionaries/lasde/entries?headword=" + word + "&part_of_speech=" + pos +
                  "&limit=1";

            var entry = JObject.Parse(HttpGET(url));
            var _out = new Dictionary<string, string>();

            if ((int)entry.SelectToken("count") < 1) {
                SendMessage("Query returned no results.");
                return;
            }

            _out.Add("word", (string)entry.SelectToken("results[0].headword"));
            _out.Add("pos", (string)entry.SelectToken("results[0].part_of_speech"));
            _out.Add("def", (string)entry.SelectToken("results[0].senses[0].definition[0]"));
            _out.Add("ex", (string)entry.SelectToken("results[0].senses[0].examples[0].text"));

            var sOut = (String.IsNullOrEmpty(_out["ex"]))
                ? _out["word"] + " [" + _out["pos"] + "] — " + _out["def"]
                : _out["word"] + " [" + _out["pos"] + "] — " + _out["def"] + " (ex. " + _out["ex"] + ")";

            SendMessage(sOut);
        }

        private void Lookup(string query, string nick) {
            var response =
                HttpGET("https://en.wikipedia.org/w/api.php?format=json&action=query&prop=extracts&exintro=&explaintext=&titles=" +
                        query);
            var pages = JObject.Parse(response)["query"]["pages"].Values().First();
            var title = (string)pages["title"];
            var content = title + " — " + Regex.Replace((string)pages["extract"], @"\r\n?|\n", "");
            var _out = StrSplit(content, 450);

            foreach (var s in _out)
                SendData("PRIVMSG", nick + " " + s);
        }

        private void Help(string nick, string cmd) {
            if (String.IsNullOrEmpty(cmd)) {
                SendData("PRIVMSG", nick + " Each command must be proceeded with \"eve\" or \"eve,\" to be properly queried.");

                foreach (var s in commands)
                    SendData("PRIVMSG", nick + " " + s.Key + ": " + s.Value);

                SendData("PRIVMSG", nick + " (* - optional parameter)");
            } else {
                SendData("PRIVMSG", nick + " " + cmd + ": " + commands[cmd]);
            }
        }

        private void Userlist() {
            var _out = new StringBuilder();

            foreach (var s in userList) {
                _out.Append(s.Name + "(" + s.Access + ") ");
            }

            SendMessage(_out.ToString());
        }

        private void Message(string sender, string who, string msg) {
            var m = new Message {
                Sender = sender,
                Contents = msg,
                Date = DateTime.UtcNow
            };

            if (QueryName(who).Messages == null)
                userList.First(e => e.Name == who).Messages = new List<Message> { m };
            else
                userList.First(e => e.Name == who).Messages.Add(m);

            SendMessage("Message recorded and will be sent to " + who);
        }

        private void Seen(string who) {
            var u = userList.FirstOrDefault(e => e.Name == who);
            SendMessage(u.Name + " was last seen on: " + u.Seen + " (UTC)");
        }
        // split input string by maxlen and return array
        public static IEnumerable<string> StrSplit(string str, int maxLength) {
            for (var i = 0; i < str.Length; i += maxLength)
                yield return str.Substring(i, Math.Min(maxLength, str.Length - i));
        }

        public User QueryName(string name) {
            return userList.FirstOrDefault(e => e.Name == name);
        }

        // check whether or not respond to user by
        // identifying the amount of commands they've
        // issued in the past minute
        public bool UserTimeout(string who) {
            var doTimeout = false;

            if (userAttempts[who] == 3)
                if (QueryName(who).Seen.AddMinutes(1) < DateTime.UtcNow)
                    userAttempts[who] = 0;
                else doTimeout = true;
            else
                if (QueryName(who).Access > 1)
                userAttempts[who] += 1;
            return doTimeout;
        }

        // initialise connection to server
        public void Connect() {
            try {
                _connection = new TcpClient(_config.Server, _config.Port);
            } catch {
                Console.WriteLine("Connection failed.");
            }

            try {
                _ns = _connection.GetStream();
                _sr = new StreamReader(_ns);
                _sw = new StreamWriter(_ns);
                _log = new StreamWriter("_logs.txt", true) { AutoFlush = true };

                Console.WriteLine(_config.Nick + " " + _config.Name);
                SendData("USER", _config.Nick + " 0 * " + _config.Name);
                SendData("NICK", _config.Nick);
            } catch {
                Console.WriteLine("Communication error.");
                throw;
            }

            userList = JsonConvert.DeserializeObject<List<User>>(File.ReadAllText("users.json"));

            if (userList == null) {
                Console.WriteLine("Failed to initialise JSON object from file");
                return;
            }
        }

        // send raw data to server
        public void SendData(string cmd, string param) {
            if (param == null) {
                _sw.WriteLine(cmd);
                _sw.Flush();
                Console.WriteLine(cmd);
            } else {
                _sw.WriteLine(cmd + " " + param);
                _sw.Flush();
                Console.WriteLine(cmd + " " + param);
            }
        }

        public void SendMessage(string message) {
            SendData("PRIVMSG", recipient + " " + message);
        }

        // send a GET request to given URL
        public string HttpGET(string url) {
            var request = (HttpWebRequest)WebRequest.Create(url);
            request.Method = "GET";

            string response;

            using (var httpr = (HttpWebResponse)request.GetResponse()) {
                response = new StreamReader(httpr.GetResponseStream()).ReadToEnd();
            }

            return response;
        }

        public void Runtime() {
            var data = _sr.ReadLine();
            var mTime = DateTime.UtcNow;
            var mRegex = new Regex(@"^:(?<Sender>[^\s]+)\s(?<Type>[^\s]+)\s(?<Recipient>[^\s]+)\s?:?(?<Args>.*)", RegexOptions.Compiled);
            var sRegex = new Regex(@"^(?<Nickname>[^\s]+)!(?<Realname>[^\s]+)@(?<Hostname>[^\s]+)", RegexOptions.Compiled);
            var aRegex = new Regex(@"^:(?<Arg1>[^\s]+)\s(?<Arg2>[^\s]+)\s(?<Arg3>[^\s]+)\s?:?(?<Arg4>.*)", RegexOptions.Compiled);
            var pRegex = new Regex(@"^PING :(?<Message>.+)", RegexOptions.None);

            Console.WriteLine(data);
            _log.WriteLine(data);

            if (mRegex.IsMatch(data)) {
                var mVal = mRegex.Match(data);
                var mSender = mVal.Groups["Sender"].Value;
                var sMatch = sRegex.Match(mSender);

                var c = new ChannelMessage() {
                    Nickname = mSender,
                    Realname = mSender,
                    Hostname = mSender,
                    Type = mVal.Groups["Type"].Value,
                    Recipient = (mVal.Groups["Recipient"].Value.StartsWith(":"))
                        ? mVal.Groups["Recipient"].Value.Substring(1)
                        : mVal.Groups["Recipient"].Value,
                    Args = mVal.Groups["Args"].Value,
                    Time = DateTime.UtcNow
                };

                if (sMatch.Success)
                {
                    var realname = sMatch.Groups["Realname"].Value;
                    c.Nickname = sMatch.Groups["Nickname"].Value;
                    c.Realname = (realname.StartsWith("~")) ? realname.Substring(1) : realname;
                    c.Hostname = sMatch.Groups["Hostname"].Value;
                }

                OnChannelMessage(c);
            } else if (pRegex.IsMatch(data)) {
                SendData("PONG", pRegex.Match(data).Value);
            }
        }
    }
}

Primary class, starts and configures program

internal class Eve {
    public static bool Run = true;

    private static void Main(string[] args) {
        var conf = new Config() {
            Name = "Evealyn",
            Nick = "Eve",
            Port = 6667,
            Server = "irc.foonetic.net",
            Channels = new[] { "#testgrounds2" },
            Joined = false
        };

        using (var bot = new IRCBot.IRCBot(conf)) {
            bot.Connect();

            while (Run) {
                bot.Runtime();
            }
        }

        Console.WriteLine("Bot has shutdown.");
        Console.ReadLine();
    }
} 

Interface IRCBot inherits from

using System;

namespace IRCBot {
    internal interface IModule
    {
        void OnChannelMessage(ChannelMessage e);
    }

    internal class ChannelMessage
    {
        public DateTime Time;

        public string
            Nickname,
            Realname,
            Hostname,
            Type,
            Recipient,
            Args;
    }

    internal class ChannelEvent
    {
        public string
            Event,
            Channel;

        public ChannelEvent()
        {

        }
    }
}

Currently, user data is structured like this:

[
  {
    "Name": "semiviral",
    "Access": 0,
    "Seen": "2015-12-12T07:35:27.1978818Z",
    "Messages": [
      {
        "Sender": "semiviral",
        "Contents": "this is a message",
        "Date": "2015-12-12T07:35:27.2198843Z"
      }
    ]
  }
]

Concerns:

  • The code is very unoptimised; rampant if-nesting, tons of methods, etc. Not very efficient or aesthetically pleasing. Solved this for a while!
  • The method of storing user objects is slow. Having to write the whole file over again with each change seems very inefficient, but I'm unsure of a superior method (This is a bit better since it now only commits one write operation per message).
  • The code is unmodular; making a change often requires a stop of the debugging process.
    • There is very little utility to the bot. Small list of commands, seems somewhat dry/worthless.

I'm certainly a fan of receiving direct feedback, but if you can also provide a resource where someone has created a very efficient/complex/powerful IRC bot I'd very much enjoy taking a look at the code. Also, suggestion of features would help quite a bit, as I'm very unimaginative myself.

The currently posted code is a second revision; I refactored it to use Regex and some far more efficient processing techniques.

\$\endgroup\$
7
  • \$\begingroup\$ Look into SignalR, it was created to abstract away this kind of problem. Of course, I understand if you implemented this as a learning exercise and didn't care to use a library. \$\endgroup\$
    – RubberDuck
    Dec 12, 2015 at 23:04
  • \$\begingroup\$ @RubberDuck Yeah, as this is a learning project I'd rather implement everything myself. If it were a real project I'd love to have some libraries to condense its code. \$\endgroup\$ Dec 14, 2015 at 10:28
  • 3
    \$\begingroup\$ I wanted to review this myself, but I don't think I'll have time, so I put up a pretty substantial bounty instead. One note, after you've received an answer, please don't update the code in your question. \$\endgroup\$
    – RubberDuck
    Dec 14, 2015 at 10:45
  • \$\begingroup\$ Thank you very much, and noted. I appreciate your assistance \$\endgroup\$ Dec 14, 2015 at 10:46
  • 2
    \$\begingroup\$ Your Config struct should be a class since it breaks the standard rules for when to use a struct (It is mutable and is larger than 16 bytes). msdn.microsoft.com/en-us/library/ms229017%28v=vs.110%29.aspx \$\endgroup\$
    – Tory
    Dec 14, 2015 at 15:32

2 Answers 2

11
+200
\$\begingroup\$

One of the key points in having readable and maintainable code is to have consistence of the choosen style which isn't the case in your code.

  • You have different indention styles for methods. Sometimes you are using K&R and sometimes you are using Allman style.
  • sometimes you are using underscore prefixed method variables (which isn't the preffered way) and sometimes you use camelCase casing to name your method variables.
  • sometimes you are using braces {} for single instruction else or for each and sometimes you don't use them.

While we are talking about braces and the use of them although they might be optional, I would like to encourage you to always use them to make your code less error prone.


Naming is another important task for having readable and understandable code. This goal will be defeated if you use abbreviation for naming variables like

private StreamWriter _log;
private NetworkStream _ns;
private StreamReader _sr;
private StreamWriter _sw;  

Seeing this variables in the code will not tell the reader of the code what they are about. Using meaningful names allows Sam the maintainer to grasp the purpose of these at first glance.


The OnChannelMessage() method is doing too many things. It is querying and adding to multiple lists, writing to file and parsing the message. You should extract parts of this method to separate methods.

For instance this

if (QueryName(c.Realname) == null
    && !nameDenyList.Contains(c.Realname)) {
    userList.Add(new User {
        Name = c.Realname,
        Access = 2,
        Seen = DateTime.UtcNow
    });
} else if (QueryName(c.Realname) != null)
    userList.First(e => e.Name == c.Realname).Seen = DateTime.UtcNow;

if (!userAttempts.ContainsKey(c.Realname))
    userAttempts.Add(c.Realname, 0);
if (!users.ContainsKey(c.Recipient)
    && c.Recipient.StartsWith("#"))
    users.Add(c.Recipient, new List<String>());  

should be in a separate method, because it doesn't has anything to do with the parsing of the message.

This magic string

case "353":  

doesn't tell what it is. Why is this not "777" ? Having magic numbers and strings in your code make it harder to understand the code. You should extract such strings (and numbers) to meaningful constants.


Based on the NET naming guidelines methods should be made out of verbs or verb phrases. You have a lot of methods which aren't following this guidelines like UserList, Message, Seen, UserTimeout and many more.

Following guidelines makes it easier for future readers of that code to understand it faster.


Instead of string concatenation like

SendMessage(u.Name + " was last seen on: " + u.Seen + " (UTC)");  

you should either use string.Format() like so

SendMessage(string.Format("{0} was last seen on: {1} (UTC)", u.Name, u.Seen));   

or if you are using C# 6 (VS 2015) you can use string interpolation by using the $ operator like so

SendMessage($"{u.Name} was last seen on: {u.Seen} (UTC)");  

Because regex expressions mostly aren't understandable at first glance it is important to name them good.

var mRegex = new Regex(@"^:(?<Sender>[^\s]+)\s(?<Type>[^\s]+)\s(?<Recipient>[^\s]+)\s?:?(?<Args>.*)", RegexOptions.Compiled);
var sRegex = new Regex(@"^(?<Nickname>[^\s]+)!(?<Realname>[^\s]+)@(?<Hostname>[^\s]+)", RegexOptions.Compiled);
var aRegex = new Regex(@"^:(?<Arg1>[^\s]+)\s(?<Arg2>[^\s]+)\s(?<Arg3>[^\s]+)\s?:?(?<Arg4>.*)", RegexOptions.Compiled);
var pRegex = new Regex(@"^PING :(?<Message>.+)", RegexOptions.None);

so this isn't helping.


Declaring multiple variables on the "same line" like in the ChannelMessage class doesn't buy you anything but costs readability.

internal class ChannelMessage
{
    public DateTime Time;

    public string
        Nickname,
        Realname,
        Hostname,
        Type,
        Recipient,
        Args;
}

Parents should talk to their children by using methods and properties but a child should talk to its parent by using events. So this

case "shutdown":
    if (access > 1)
    {
        SendMessage("Goodybe.");
        Eve.Run = false; // <- this
    }

is not good.

\$\endgroup\$
0
10
\$\begingroup\$

I'm not a C# expert, so I'll try my best to give a good review:


        {"chanlist", "Provides a list of channels joined."},
        {"channels", "Provides a list of channels joined."},

These functions are identically described, either make them different or remove one.


recipient: you don't need this field. It's only used in one function, and it could be passed in as a parameter at best.


In the following code block, you have a magic number (an undefined, unknown number):

            userList.Add(new User {
                Name = c.Realname,
                Access = 2,
                Seen = DateTime.UtcNow
            });

You should define what 2 means in relation to access as a constant or in an enum for clarity, so people can understand what each type of access means.


You've left out brackets on quite a few of your if statements. While you might see this as 'unnecessary formatting', it's better to use brackets around them. See the Apple SSL bug.


The following block is incorrectly formatted, and can be improved:

                if (arg[0] != "eve"
                    && arg[0] != "eve,")
                    return;

into:

if (arg[0].replace(",", string.Empty) != "eve")
{
    return;

Perhaps this ought to be a constant up the top for easier maintainability:

"Evealyn Bot is a sophisticated and damn fine IRC bot created by SemiViral. Version 1.0"

The indentation is off here:

            case "JOIN":
        Console.WriteLine(c.Realname);
\$\endgroup\$
3
  • \$\begingroup\$ recipient is used in SendMessage() as well. The mentioned strings at the top of your answer are key value pairs of a dictionary. So if the key is needed the value might be the same. \$\endgroup\$
    – Heslacher
    Dec 14, 2015 at 14:27
  • 1
    \$\begingroup\$ Yes, @Heslacher, that's why I said: It's only used in one function, and it could be passed in as a parameter at best. \$\endgroup\$
    – Quill
    Dec 14, 2015 at 14:28
  • \$\begingroup\$ I use recipient as a global var because SendMessage is referenced everywhere, and it's often with the same parameter(c.Receipient), so I set that to a global var rather than typing it everywhere. As for the rest, I've taken the advice and made the changes you suggested. Thank you \$\endgroup\$ Dec 14, 2015 at 14:31

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.