Re: Package Proposal: Math_RPN

The name should be Math_RPN (as Martin pointed out too) The code looks nice, just some ideas/comments: 1) This class takes an infix expression and converts it into an RPN (postfix) expression, it does not evaluate an RPN expression. I would have setExpression() that has a second param with a boolean to indicate whether is an infix or an RPN expression (although it can be figured out during parsing too). 2) The getValue() method perhaps should be names calculate() or evaluate() instead (just a suggestion), and might use the setExpression() mentioned above. 3) Usually private elements have an underscore at the beginning of their names, so some renaming will be needed, e.g. $error to $_error 4) Apart from s/pile/stack/, another one would be s/Angle's gauge/Angle's unit/ (one speaks of 'unit conversion' in english, not 'gauge conversion'), also perhaps s/rule/_operation/ 5) Arnaud's idea of adding a new rule/operation would be nice, perhaps and addOperation() method that points to a callback? The callback can be a function or a methods in a class/object. 6) Not sure if the PHP token_get_all() function will help you make your _stringToArray(), etc. parsing methods simpler, e.g.: $exp = "2+(2*6)"; $tokens = token_get_all("<?php $exp; ?>"); print_r($tokens); 7) Instead of using array_keys() and then in_array() in your method _keyExists(), you could use the array_key_exists() function (has been in PHP since 4.1.0) 8) Usually exp(x) is equivalent to e^x not to 10^x, so that will need to be changed in your code. And, if you are going to accept scientific notation (where 1E-5 means 10^-5), you will have to change your parsing of the neperian number whose symbol is 'e' not 'E'. Also, why are you using exp(1) for e when there is a constant M_E defined in PHP? similar argument goes for M_PI, etc. The code looks like it follows an algorithm like the one described in: http://www.cs.laurentian.ca/dgoforth/cosc2006/asst4/algorithm_to_convert_infix.html I just had a look at the code, had not tested it yet (new laptop, still setting up stuff so no PHP in it for now). Bottomline: a conditional +1, depending on the comments above. If you have a good justification that overrides one or more of the comments/arguments, I'll be happy to see them. --- Maciej Szczytowski <admin@e-rower.pl> wrote: > Hi, > I wrote a class that can make and calculate a RPN (reversed polish > notation) expression. > RPN is formal logic system which allowed mathematical expressions to be > specified without parentheses by placing the operators before or after the > operands. > For example 2+5*7 in RPN looks 2,5,7,*,+ > > You can take a look at the source code and example here: > http://www.maciek.maribex.wizja.net.pl/rpn/ > > Sample usage: > > $expression = '(3+2)*(7-sin(30)*log(10))'; > > require_once 'Math/RPN.php'; > > $rpn = new RPN(false); > > $value = $rpn->getValue($input); // 32.5 > > Any questions, comments, or suggestions? > > Regards, > Maciek Szczytowski > admin@e-rower.pl ===== --- Jesus M. Castagnetto (jcastagnetto@yahoo.com) Research: http://metallo.scripps.edu/ Personal: http://www.castagnetto.org/ __________________________________ Do you Yahoo!? The New Yahoo! Search - Faster. Easier. Bingo. http://search.yahoo.com