4
\$\begingroup\$

So I was thinking of making a C# REPL, as I didn't find any viable solution that would work for me, so I made a simple, basic one, I am sure it can be improved, as it seems once the REPL class is initialized it's hard to add an event handler to it, or access the logs, so I am looking for a way to improve it, here's the code:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace CS_REPL
{
    public class REPL
    {
        List<string> inputLog;

        public delegate void InputReceived(string input);
        public event InputReceived InputHandler;

        private bool loopEnabled = true;

        public REPL(InputReceived handler)
        {
            inputLog = new List<string>();
            InputHandler += handler;
            while (loopEnabled)
            {
                string input = ReadLine();
                inputLog.Add(input);
                InputHandler?.Invoke(input);
            }
        }

        public string ReadLine()
        {
            PrintShellIndicator();
            return ReadInput();
        }

        public void PrintShellIndicator()
        {
            Console.Write("> ");
        }

        public string ReadInput()
        {
            return Console.ReadLine();
        }

        public void setRunning(bool flag)
        {
            loopEnabled = flag;
        }
    }
}
\$\endgroup\$
3
\$\begingroup\$

once the REPL class is initialized

You cannot initialize or rather create an instance of REPL becasue there is an infinite loop in the constructor - the by default you set loopEnabled = true;. Is this intended?

it's hard to add an event handler to it

It's hard because there is no object to work with as the new REPL(...) will never return so you can only add the handler once via the constructor.

Code

The delegate shouldn't be nested inside the REPL class. It also shouldn't be a custom delegate too - if used as an event. By convention we use the EventHandler delegate for this.

As you want to pass the input to the event handler you'll need to create custom EventArgs to make it possible. There is a generic EventHandler<TEventArgs> where you can use the new class.

public class InputReceivedEventArgs : EventArgs
{
    public InputReceivedEventArgs(string input)
    {
        Input = input;
    }

    public string Input { get; }
}

The final REPL class with the new event could look like this:

public class REPL
{
    private List<string> _inputLog;

    // Initialize with an empty event handler.
    public event EventHandler<InputReceivedEventArgs> InputReceived = delegate { };

    public REPL()
    {
        _inputLog = new List<string>();
    }

    // Starts the REPL
    public void Start()
    {
        while (true)
        {
            string input = ReadLine();
            _inputLog.Add(input);
            OnInput(input);
        }
    }

    // --- all other methods should be private

    private string ReadLine()
    {
        PrintShellIndicator();
        return ReadInput();
    }

    private void PrintShellIndicator()
    {
        Console.Write("> ");
    }

    private string ReadInput()
    {
        return Console.ReadLine();
    }

    // Invokes the InputReceived event
    private void OnInput(string input)
    {
        InputReceived.Invoke(this, new InputReceivedEventArgs(input));
    }
}
\$\endgroup\$
13
  • \$\begingroup\$ why do you initialize InputReceived with an empty handler? \$\endgroup\$ – user73941 Sep 27 '16 at 7:51
  • \$\begingroup\$ @HenrikHansen to avoid the null-check. \$\endgroup\$ – t3chb0t Sep 27 '16 at 8:03
  • \$\begingroup\$ But isn't the InputReceived?.Invoke() optimized for that or what is most expensive? \$\endgroup\$ – user73941 Sep 27 '16 at 8:07
  • 1
    \$\begingroup\$ @HenrikHansen there are various opinions, I like the empty handler, see Is it a good practice to define an empty delegate body for a event? \$\endgroup\$ – t3chb0t Sep 27 '16 at 8:17
  • \$\begingroup\$ OK, it's a matter of taste - I don't like that taste ;-) \$\endgroup\$ – user73941 Sep 27 '16 at 8:28

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.