Code review comment for lp:~allenap/storm/value-columns-by-name

Gavin,

I don't quite see the motivation for this feature. Using the actual class attributes is how it works pretty much everywhere else in Storm, and doesn't look like a big burden when compared to the column names.

If you have a huge class name, you can easily do something like this:

    C = MyHugeClassName

And then have exactly the same call length:

    resultset.values(C.id, C.name)

vs.

    resultset.values("id", "name")

With the advantage that the former, in case of errors, will blow up as syntax errors early, rather than SQL exceptions.

The import fascist isn't a great reason to add support like this in Storm either. Having a result set in the code means you have access to the model objects. What would be the reason of preventing access to the object's class in a situation where you have the object and a *result set* (which allows one to change column values without any checks).

Then, there's a significant additional cost being introduced in a place which used to be pretty lightweight.

For these reasons, I don't feel like it's an improvement over how it works today.