Evaluating algebra expression using an auxiliary VB class compiled on the fly and consumed in a C# project

Question

• I want to create algebra evaluator but I don't want to write my own parser because it takes much time to learn.

• I don't like algebra expression in C# because it does not look natural compared to VB counterpart. For example, "x^2+1" in VB must be written as "Math.Pow(x,2)+1" in C#.

• I am still a newbie in the world of programming and always interested to learn the best practices. I am always not confident with my code.

Minimal Working Example

Based on the 3 constraint given above, I tried to implement it as follows.

First, I create a template named template.vb as follows:

Imports System
Imports System.Math
Namespace AnyNamespace
    Public Class AnyClass
        Public Function AnyMethod(ByVal x As Double) As Double
            Return expression-in-x
        End Function
    End Class
End Namespace

where expression-in-x is just a place holder that will be replaced. See the next code how I replace it.

Second, I create a class named Parser.cs as follows:

using Microsoft.VisualBasic;
using System;
using System.CodeDom.Compiler;
using System.IO;
using System.Reflection;
using System.Text;


namespace Mathematics
{
    public static class Parser
    {
        static string expression;
        static object obj;
        static MethodInfo mi;

        public static string Expression
        {
            get
            {
                return expression;
            }
            set
            {
                expression = value;

                string source;

                using (StreamReader sr = new StreamReader("template.vb"))
                {
                    source = sr.ReadToEnd().Replace("expression-in-x", expression);
                }

                CompilerParameters cps = new CompilerParameters();
                cps.GenerateExecutable = false;
                cps.GenerateInMemory = true;

                CompilerResults crs = (new VBCodeProvider()).CompileAssemblyFromSource(cps, source);

                if (crs.Errors.Count > 0)
                {
                    StringBuilder sb = new StringBuilder();
                    foreach (CompilerError ce in crs.Errors)
                    {
                        sb.Append(string.Format("Error Number : {0}, Line : {1}, Message : {2}.\n", ce.ErrorNumber, ce.Line, ce.ErrorText));
                    }
                    throw new Exception(sb.ToString());
                }
                else
                {
                    Assembly a = crs.CompiledAssembly;
                    obj = a.CreateInstance("AnyNamespace.AnyClass");
                    mi = obj.GetType().GetMethod("AnyMethod");
                }
            }
        }



        public static double Evaluate(double x)
        {
            if (expression == null)
                throw new Exception("Unitialized expression.");
            try
            {
                return (double)mi.Invoke(obj, new object[] { x });
            }
            catch (TargetInvocationException error)
            {
                throw error;
            }
            catch (Exception error)
            {
                throw error;
            }
        }
    }
}

Last, I test the code as follows:

using Mathematics;
using System;

namespace Tester
{
    class Program
    {
        static void Main(string[] args)
        {
            Parser.Expression = "x^2+1";
            for (int x = 0; x < 10; x++)
                Console.WriteLine(Parser.Evaluate(x));
            Console.ReadKey();
        }
    }
}

Questions

Is what I did above recommended? If not, could you give me a direction or suggestion what should I improve it or whatever to make it much better?

Answer 1

public static class Parser

Why is this class static? When you have a class with some state, you should almost never make that state static. For example, you might be very surprised when you try using this code from multiple threads.

---

using (StreamReader sr = new StreamReader("template.vb"))
{
    source = sr.ReadToEnd().Replace("expression-in-x", expression);
}

You could have used File.ReadAllText() to make this simpler.

---

CompilerParameters cps = new CompilerParameters();
cps.GenerateExecutable = false;
cps.GenerateInMemory = true;

Using var and object initializer, you can shorten this to:

var cps = new CompilerParameters { GenerateExecutable = false, GenerateInMemory = true };

Also, cps is not a great name, don't shorten names unnecessarily.

---

obj = a.CreateInstance("AnyNamespace.AnyClass");
mi = obj.GetType().GetMethod("AnyMethod");

Instead of object and MethodInfo, you could store the method as a delegate:

Func<double, double> compiledFunction;

…

var obj = a.CreateInstance("AnyNamespace.AnyClass");
var mi = obj.GetType().GetMethod("AnyMethod");
compiledFunction =
    (Func<double, double>)Delegate.CreateDelegate(typeof(Func<double, double>), obj, mi);

…

return compiledFunction(x);

As another advantage, this is likely going to be faster, since it's not using reflection for every invocation of Evaluate().

Answer 2

Is what I did above recommended?

Parts of it yes, other parts no.

Using the using keyword is good.
Using a StringBuilder to append/concat Strings is good.

but I don't want to write my own parser

But you name your class Parser, which is misleading.

    public static double Evaluate(double x)
    {
        if (expression == null)
            throw new Exception("Unitialized expression.");
        try
        {
            return (double)mi.Invoke(obj, new object[] { x });
        }
        catch (TargetInvocationException error)
        {
            throw error;
        }
        catch (Exception error)
        {
            throw error;
        }
    }  

First you are catching the exception and then you are throwing it. If you can't handle the exception, you shouldn't catch it. Also using your style throw error will discard all the useful information of the stacktrace. In the case you really need to rethrow an exception, but you need to add some information, you should create a new exception and pass the catched exception as the inner exception to the constructor like

try
{
    return (double)mi.Invoke(obj, new object[] { x });
}
catch (TargetInvocationException error)
{
    throw new TargetInvocationException("Evaluate has thrown an exception because blabla...", error);
}

Brackets should be used also for "oneline" expressions of loops and if so

for (int x = 0; x < 10; x++)
      Console.WriteLine(Parser.Evaluate(x));  

should be

for (int x = 0; x < 10; x++)
{
    Console.WriteLine(Parser.Evaluate(x));
}  

Your Expression setter is doing to much. You should refactor the loading and compiling of your assembly to a method. For the loading you should also refactor the Errormessage composition to a separate method.

Last but not least, I want to suggest that you should create a real expression parser. You will learn a lot by this experience.