Hello Federico,
> this is the first of a series of patches vs the System.Linq.Expressions
> namespace. This patch doesn't change a lot of things but at least adds
> some tests (they were completely missing before). Included:
>
> * Added Test/ directory, modified Makefile to build and execute them
> * Changed a couple of Expression methods to raise exceptions identicals
> to MS ones
> * Added tests for AddExpression and ConstantExpression
> * Implemented somme missing stuff in BinaryExpression
> * The stuff in ExpressionUtils is very generic and does quite some
> redundant checks: I started splitting the stuff there into more
> "specific" methods that should be both understandable and fast.
>
Nice!
Just a few minor problems:
* Assert.AreEqual (expr.Method, null);
It's always better to use most appropriate method. In the cases like
this should be
Assert.IsNull or similar.
* Assert.AreEqual(expr.Method.Name, "op_Addition");
The preferred way it's add a message or at least id to each assertion.
* BindingFlags flags = BindingFlags.Public | BindingFlags.NonPublic |
BindingFlags.Static;
This should be constant.
* builder.Append ("[" + nodeType + "]");
I know you didn't write this code, but this is preferred when you have
StringBuilder
to avoid double allocation.
builder.Append ("[").Append (nodeType).Append ("]");
> Also, as I check that everything is done I do some cosmetic changes to
> have the code in line with the style guidelines. Hope this is ok.
>
Yes, no problem.
> Just tell me if the patch is fine and if does make sense to continue
> implementing System.Linq.Expressions.
>
>
Thanks,
Marek
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
