Code review comment for lp:~zorba-coders/zorba/bug1147563

> I still think that the overhead of an extra call makes the code more readable
> and it's worth it in the long run. Still the nillable is not a property of the
> type of the element but of the element itself.
>
> Please leave the create word in "createXQTypeFromElementName", these methods
> have side effects they aren't just simple gets and I think a good name should
> tell this. A more correct name for this method would be:
> createXQTypeFromElementNameAndGetNillable. But I still think makes more sense
> to have them separate and leave the compiler do the optimizations for this
> case.
>
> At least leave createXQTypeFromElementName name if you think the first part
> has a significant effect on performance.

I am sorry but I disagree. The callers of these methods want to get both type info (either just the name or a full XQType) and the nillable property from a global declaration. Why make two calls for this, when the two calls make almost the same work underneath? Maybe it doesn't make too much difference in the overall query performance, but it's not good coding practice, IMO. I can rename the methods to "getInfoFromGlobalElementDecl" if you think that makes them more readable. And if in the future a new caller needs only the type info or only the nillable info, we can add new methods that return just the needed info.

By the way, in validate.cpp line 207 and 268 there are calls to getTypeInfoFromGlobalElementDecl() that look like noops to me, because they just discard the returned XQType and do not raise any exception if the declaration is not found. Do I miss something?