current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote -2 down vote favorite

I have a string like str = " 4+ 6 * 30";. I have to perform an arithmetic operation on this using C#; My solution to this problem is as follows.

     string temp = " 4 + 6 * 5";
     int firstNaum = 0;
     int secondNum = 0;
     int ThirdNum = 0;
     int finalResults = 0;
    //Spliting strings
    string[] withoutOperator = temp.Split('\t',' ','*' , '+');
    //Iterating strings 
    int counter = 0;
    foreach (var res in withoutOperator)
    {
        if (!string.IsNullOrEmpty(res) && counter ==1)
        {
            firstNaum = Convert.ToInt32(res);
        }
        if (!string.IsNullOrEmpty(res) && counter== 4)
        {
            secondNum = Convert.ToInt32(res);
        }
        if (!string.IsNullOrEmpty(res) && counter == 7)
        {
            ThirdNum = Convert.ToInt32(res);
        }
        counter += 1;
    }
    finalResults = firstNaum + secondNum * ThirdNum;
share|improve this question
4  
The process that you're using is very tightly coupled to your example, and not at all extensible to other input strings. – krillgar Jul 24 '14 at 16:17
up vote 8 down vote

Your solution is very limited. As it only take a string in in the following format x1 x2 x3, and apply to the function x1+x2*x3.

To build an expressional calculator, you need to go through these steps :

  1. Segmentate the expression into a list of tokens
  2. Parse the tokens into recognizable lexemes(numbers, operator, parenthesis, etc)
  3. Transform the list into a tree structure based on the order of operations
  4. Calculate the result of top root, which requires the results from its branch(es)
share|improve this answer
2  
However, such expressions are extremely easy to parse and calculate, as the Shunting-Yard algorithm can be used to evaluate the tokens. So of your four steps, only two remain: tokenization (I don't get why you split that into two phases), and evaluation via Shunting-Yard. – amon Jul 24 '14 at 16:41
up vote 5 down vote

Can you clarify what you want us to feed back on.

My thoughts are:

  1. Use a library that already performs infix mathematical parsing because the process is relatively complex but has been "solved" many times before.
  2. Your code assumes a very strict order in the the operations. That doesn't make it very extensible.
  3. Try adding validation. instead of Convert.ToInt32(res), use int.TryParse(res, out n). if the function returns false, you know the parse failed and you can handle it as you feel fit.
  4. Maybe Trim() your res on each loop - although you will need to assign it to a new variable (e.g. var res2 = res.Trim();)
share|improve this answer
up vote 2 down vote

One of the simplest and easiest ways to evaluate math expressions is to use the Compute method of the DataTable class:

DataTable dt = new DataTable();
int answer = (int)dt.Compute("2+(4*3)*4", "");

answer is 50.

One caveat you'll have to trap exceptions to catch invalid expressions.

share|improve this answer
up vote -1 down vote

Using the solution you started will be soon or later way more complex when you will have to support operator precedence and parenthesis.

Disclaimer: I'm the owner of the project Eval Expression.NET on GitHub

This library is the easiest way to dynamically evaluate arithmetic string at runtime and support almost all the C# Syntax.

Using your example

// return 34
var result = Eval.Execute<int>("4 + 6 * 5");
share|improve this answer
1  
Whilst what you are suggesting is a very nice solution, it doesn't really review the code beyond "there's a library for that". – Pimgd Feb 12 at 13:54
    
@Pimgd Which is an entirely valid review, last time I checked. – Simon Forsberg♦ Feb 12 at 14:01
    
@SimonForsberg Then it's okay =) I wasn't entirely sure - Personally I didn't downvote. – Pimgd Feb 12 at 14:02
    
This is just a link to your library with no justification. let alone the fact that it's a paid library. Flagging as spam. – Quill Feb 12 at 14:06
    
I used the example from the question, it's free up to 50 characters (which is more then enough for almost all expression). What else can I do? I will be happy to edit my answer but he has a simple question and I write a simple answer so I'm not sure what I can add for not be a "SPAM" – Jonathan Magnan Feb 12 at 14:19

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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