On Sat, May 10, 2008 at 9:47 PM, Sebastian Noack
<[EMAIL PROTECTED]> wrote:
> On Sat, 10 May 2008 20:36:14 +0800
> "Russell Keith-Magee" <[EMAIL PROTECTED]> wrote:
>> * I don't see why LiteralExpr is required. Surely a literal should
>> continue to work as is; if a literal is involved in an expression with
>> a ColumnExpr(), the definition of ColumnExpr should be smart enough to
>> merge the literal into the resultant expression.
>
> You can still do model.objects.update(foo=42) with my patch, because
> of 42 is casted to a LiteralExpr under the hood. I could even make it
> possible to do model.objects.update(foo=CurrentExpr() + 42). But there
> is no way to enable model.objects.update(foo=42 + CurrentExpr()),
> because of in this case int.__add__ instead of Expression.__add__ is
> called, so I decided to introduce LiteralExpr. But I agree that it
> would be cool, if you could use a literal as it is.
I figured that was probably the reason. It's a bit of an edge case,
but I can see where it will be required, especially with division
(since you can't reverse the operands).
As a matter of style, I would suggest calling the clause "Literal"
rather than "LiteralExpr" - a constant isn't really an expression,
except in the context of Church numerals. L() is another option, but
is perhaps a little _too_ brief for a relativey infrequent edge case.
>> * foo=CurrentExpr() is really just the same as foo=ColumnExpr('foo'),
>> isn't it? Why include two ways of providing the reference?
>
> Following statements are indeed equivalent:
>
> model.objects.update(foo=CurrentExpr() + LiteralExpr(1))
> model.objects.update(foo=ColumnExpr('foo') + LiteralExpr(1))
>
> But first is less code, because of you don't have to specify the column
> and I think second does violate the DRY slightly.
Less code is not the metric you should use. Clarity is the more
important feature. Sure, clarity and code length often coincide, but
there comes a point where you can say too little, and end up being
ambiguous or confusing.
Also - it's only a violation of DRY if you are specifying the same
thing twice. In this case, you have to specify two different things -
a source and a target. The fact that they both happen to be 'foo' is
an interesting fact about the query, not a duplication of effort.
> But the really
> interesting thing is that you can do following:
>
> increment_expr = CurrentExpr() + LiteralExpr(1)
> model_1.objects.update(foo=increment_expr)
> model_1.objects.update(bar=increment_expr)
> model_2.objects.update(foo=increment_expr)
I'm not sure I'm convinced that this is a good idea. Explicit is
better than implicit.
> I also don't like the name *Expr. I already considered to use something
> similar to Q(), but I think this is to confusing as long we have
> multiple Expression objects. If we will decide to drop LiteralExpr and
> CurrentExpr, I would like to use F(), as you proposed instead of.
I have no problem dropping CurrentExpr(). This leaves "Literal()" and
"F()", which seems like a manageable API to me.
>> On top of these criticisms, there is at least one very good reason
>> that this patch won't get merged as-is - it doesn't contain any
>> documentation or test cases. What you are proposing is a fairly big
>> feature addition - documentation and tests would be mandatory before
>> the patch was accepted. I can understand why you might have left off
>> documentation until your idea was accepted, but I (and the other core
>> developers) are rarely convinced that code works unless there is a
>> comprehensive test suite to back it up.
>
> I know this. But there is no sense do this at the moment. As you
> already noted there are still a few points to discuss and probably
> things to change on this code. So I will write documentation and tests
> when this is finished, because of otherwise I have to do this work
> twice.
This is true for documentation, but not for test cases. How do you
know that your code works? How do I know that your code works? How do
I know how comprehensive you have been in your personal testing?
Tests should be the first thing you write, not the last. If nothing
else, writing the tests can help to clarify the ideas that you have
and help to identify the difficulties you may experience during
development.
Yours,
Russ Magee %-)
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~----------~----~----~----~------~----~------~--~---
