4
\$\begingroup\$

I'm an active contributing member of Wiktionary and an active member of an active Wiktionary Discord server. Some time ago, several users proposed a bot which would make linking to Wiktionary in Discord much easier (originally one would have had to go to Wiktionary, search their entry, copy the link, and then navigate back to Discord to paste it); thus I made this bot.

The bot works exactly like link formatting on Wikimedia sites; the only exception is that the heading character # is replaced with ? since Discord already utilizes pound signs to reference server channels. A couple of example commands might be:

  • [[cogito?Latin|cōgitō]] produces cōgitō: https://en.wiktionary.org/wiki/cogito#Latin
  • [[cogito?Latin]] produces https://en.wiktionary.org/wiki/cogito#Latin
  • [[cogito|cōgitō]] produces cōgitō: https://en.wiktionary.org/wiki/cogito
  • [[cogito]] produces https://en.wiktionary.org/wiki/cogito
  • etc.

The bot can also produce links to other Wikimedia sites, like Wiktionary, Wikibooks, etc. due to the fact that top-level namespaces exist on every Wikimedia site which serve as redirects. Examples:

  • [[wikipedia:Tire?History]] produces https://en.wiktionary.org/wiki/wikipedia:Tire#History
  • [[wikipedia:Tire?History|History of the tyre]] produces History of the tyre: https.en.wiktionary.org/wikipedia:Tire#History
  • etc.

Commands can also be embedded within the middle of messages; one could write something like The word [[subductisupercilicarptor?Latin]] is one of Latin's longest and the bot would be able to output the message https://en.wiktionary.org/wiki/subductisupercilicarptor#Latin.

EntryPoint.cs*

using System;
using System.Linq;
using System.Threading.Tasks;
using System.Collections.Generic;
using System.Text.RegularExpressions;
using Discord;
using Discord.WebSocket;

namespace WiktionaryBot
{
    /// <summary>
    /// Entry point of the bot.
    /// </summary>
    public class EntryPoint
    {
        const string CLIENT_ID = "REDACTED";
        const string CLIENT_SECRET = "REDACTED";
        const string BOT_TOKEN = "REDACTED";

        /// <summary>
        /// Normal entry point of the application, starts an async context.
        /// </summary>
        /// <param name="args">The command line arguments passed to the bot.</param>
        public static void Main(string[] args)
            => new EntryPoint().MainAsync(args).GetAwaiter().GetResult();

        /// <summary>
        /// The async context of the bot.
        /// </summary>
        /// <param name="args">The command line arguments passed to the bot.</param>
        public async Task MainAsync(string[] args)
        {
            DiscordSocketClient discordClient = new DiscordSocketClient();
            await discordClient.LoginAsync(TokenType.Bot, BOT_TOKEN);
            await discordClient.StartAsync();

            discordClient.MessageReceived += this.MessageRecieved;

            await Task.Delay(-1);
        }

        /// <summary>
        /// This function is triggered when a message is received.
        /// </summary>
        /// <param name="message">The message received.</param>
        public async Task MessageRecieved(SocketMessage message)
        {
            MatchCollection matchCollection = Regex.Matches(message.Content, @"\[\[[^\[\]]+\]\]");
            List<string> matchedStrings = matchCollection.Cast<Match>().Select(match => match.Value).ToList();

            string finalLinkString = "";
            foreach(string matchedString in matchedStrings)
            {
                CommandParser commandParser = new CommandParser(matchedString);
                commandParser.GenerateTokens();
                commandParser.GenerateOutputLink();

                finalLinkString += commandParser.OutputText[0].Item2 + "\n";
            }

            await message.Channel.SendMessageAsync(finalLinkString);
        }
    }
}

CommandParser.cs

using System;
using System.Linq;
using System.Collections.Generic;
using System.Text.RegularExpressions;

namespace WiktionaryBot
{
    /// <summary>
    /// This enum contains all types of command tokens.
    /// </summary>
    public enum CommandTokenType
    {
        LinkTitle,
        LinkSubheading,
        LinkDisplayTitle
    }

    /// <summary>
    /// This enum contains all of the different types of command output.
    /// </summary>
    public enum CommandOutputType
    {
        Link,
    }

    /// <summary>
    /// This class is responsible for parsing an input command.
    /// </summary>
    public class CommandParser
    {
        public string InputText { get; set; }
        public List<Tuple<CommandOutputType, string>> OutputText { get; set; }
        private List<Tuple<CommandTokenType, string>> OutputTokens { get; set; }

        /// <summary>
        /// Constructor for the CommandParser class.
        /// </summary>
        /// <param name="inputText">The input command.</param>
        public CommandParser(string inputText)
        {
            this.InputText = inputText;
            this.OutputText = new List<Tuple<CommandOutputType, string>>() { };
            this.OutputTokens = new List<Tuple<CommandTokenType, string>>() { };
        }

        /// <summary>
        /// Helper method for this.GenerateTokens.
        /// </summary>
        /// <param name="splitString">A reference to the splitString list.</param>
        private void GenerateTokens(ref List<string> splitString)
        {
            splitString.AddRange(Regex.Split(splitString[0], @"(?=[\?\|])"));
            splitString.RemoveAt(0);

            this.OutputTokens.Add(
                new Tuple<CommandTokenType, string>(
                    CommandTokenType.LinkTitle,
                    splitString[0]
                )
            );

            for(int i = 1; i < splitString.Count; i++)
            {
                string currentToken = splitString[i];
                switch(currentToken[0])
                {
                    case '?':
                        this.OutputTokens.Add(
                            new Tuple<CommandTokenType, string>(
                                CommandTokenType.LinkSubheading,
                                splitString[i].Trim('?')
                            )
                        );
                        break;

                    case '|':
                        this.OutputTokens.Add(
                            new Tuple<CommandTokenType, string>(
                                CommandTokenType.LinkDisplayTitle,
                                splitString[i].Trim('|')
                            )
                        );
                        break;

                    default:
                        break;
                }
            }
        }

        /// <summary>
        /// Generate a list of tokens to be interpreted.
        /// </summary>
        public void GenerateTokens()
        {
            string linkText = this.InputText.Trim('[', ']');
            List<string> splitString = new List<string>() { linkText };
            this.GenerateTokens(ref splitString);
        }

        /// <summary>
        /// Generate an output link based on the generated output tokens.
        /// </summary>
        public void GenerateOutputLink()
        {
            string linkTitle = this.OutputTokens
                .Where(token => token.Item1 == CommandTokenType.LinkTitle)
                .ToList()[0].Item2.Replace(' ', '_');

            string linkSubheading = this.OutputTokens.Any(token => token.Item1 == CommandTokenType.LinkSubheading)
                ? "#" + this.OutputTokens.Where(token => token.Item1 == CommandTokenType.LinkSubheading).ToList()[0].Item2.Replace(' ', '_')
                : "";

            string linkDisplayTitle = this.OutputTokens.Any(token => token.Item1 == CommandTokenType.LinkDisplayTitle)
                ? this.OutputTokens.Where(token => token.Item1 == CommandTokenType.LinkDisplayTitle).ToList()[0].Item2 + ": "
                : "";

            this.OutputText.Add(
                new Tuple<CommandOutputType, string>(
                    CommandOutputType.Link,
                    String.Format("{0}{1}{2}{3}", linkDisplayTitle, "https://en.wiktionary.org/wiki/", linkTitle, linkSubheading)
                )
            );
        }
    }
}

The GitHub link for this bot can be found here; any suggestions for improvements are welcome.

* The CLIENT_ID, CLIENT_SECRET, and BOT_TOKEN values are redacted in EntryPoint.cs; they are private values used by Discord to connect and authenticate the bot.

\$\endgroup\$
4
\$\begingroup\$

There are a couple of things that you can change to simplify the code and to greatly improve the readability.

public static void Main(string[] args)

If you are working with the latest C# then it's now legal to make Main async:

public static async Task Main(string[] args) { .. }

This means you don't need two Mains.


You are using tuples a lot and declaring them with Tuple<,> like

public List<Tuple<CommandOutputType, string>> OutputText { get; set; }

or creating them with new Tuple<,>

this.OutputTokens.Add(
    new Tuple<CommandTokenType, string>(
        CommandTokenType.LinkTitle,
        splitString[0]
    )
);

Neither of those is necessary. It is now possible to use a much shorter syntax like respectively:

public List<(CommandOutputType, string)> OutputText { get; set; }

where you can get rid of the Tuple and just use the ()

this.OutputTokens.Add(
    (
        CommandTokenType.LinkTitle,
        splitString[0]
    )
);

but you can do even more. You can name each field!

public List<(CommandOutputType Type, string Text)> OutputText { get; set; }

also since this is a collection you should name this property propertly. Not OutputText but Outputs (plural) - now that the tuples contain the name Text I find it's no longer necessary to use Text as a part of the property name as this would be redundant.


public async Task MessageRecieved(SocketMessage message)
{
    MatchCollection matchCollection = Regex.Matches(message.Content, @"\[\[[^\[\]]+\]\]");
    List<string> matchedStrings = matchCollection.Cast<Match>().Select(match => match.Value).ToList();

    string finalLinkString = "";
    foreach(string matchedString in matchedStrings)
    {
        CommandParser commandParser = new CommandParser(matchedString);
        commandParser.GenerateTokens();
        commandParser.GenerateOutputLink();

        finalLinkString += commandParser.OutputText[0].Item2 + "\n";
    }

    await message.Channel.SendMessageAsync(finalLinkString);
}

It's not necessary to put the matched string is a List<> first. Let it be an IEnumerable<> and let the foreach execute it. Otherwise your iterating it twice. This might not matter here with a small number of items but this looks like a bad habit and can cause loss of performance somewhere else.

Using the += for building a string inside a loop is another bad habit. Strings are immutable and this might cost a lot of memory with larger strings. You should consider using the StringBuilder or collect the partial strings in a list and use string.Join after you have them all.

Here we can also see the ugly .Item2 property of the tuple. If you declare the OutputText (or as suggested Outputs) with the new syntax the you'll be able to write:

commandParser.Outputs[0].Text

I don't like the design of the CommandParser. It should not be working like it's doing now, this means that you need to create an instance of it first, then you call some methods that don't return any values and where you need to call them in a specific order. Instead, you should create the parser only once and have a method that returns the result immediately.

Ideally I imagine the above example should go something along these lines:

public async Task MessageRecieved(SocketMessage message)
{
    var commandParser = new CommandParser(); // or as class' field

    var matchedString = 
        Regex
            .Matches(message.Content, @"\[\[[^\[\]]+\]\]");
            .Cast<Match>()
            .Select(match => match.Value);

    // You don't need to split it in two queries but might be helpful for debugging.

    var links =
        matchedStrings
            .Select(matchedString => commandParser.Parse(matchedString).Text);

    var finalLinkString = string.Join("\n", links) + "\n"; // if you really need that last "\n"            

    await message.Channel.SendMessageAsync(finalLinkString);
}

You're using ToList()[0] in a lot of places. Always with the index 0. This screams for a different design where OutputText and OutputTokens (notice the inconsistent naming) shouldn't be lists at all but probably just simple tuples.


I also think you should use var more and this less. It'll make your code less verbose. The above suggestions apply to many other places so I won't comment on them.

I'd be a good idea to redesign the CommandParser's API to make its usage more intuitive.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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