sonar-openedge
sonar-openedge copied to clipboard
Multiple buffers in one query
In the ABL you can have multiple buffers in one query. This can lead to fun problems when you, for example, refactor to local buffers and forget one line. The following example never hits the message since the global ttFoo Buffer is filled with the last created record. A query should just use one buffer on the left side. Joins are of course a bit different since a join are multiple queries, but still, each query should just use one buffer on the left side.
DEFINE TEMP-TABLE ttFoo NO-UNDO
FIELD cField1 AS CHARACTER
FIELD cField2 AS CHARACTER
.
CREATE ttFoo.
ASSIGN
ttFoo.cField1 = "1":U
ttFoo.cField2 = "2":U
.
CREATE ttFoo.
ASSIGN
ttFoo.cField1 = "3":U
ttFoo.cField2 = "4":U
.
DEFINE BUFFER lbFoo FOR ttFoo.
FOR EACH lbFoo WHERE
lbFoo.cField1 = "1":U AND
ttFoo.cField2 = "2":U
NO-LOCK:
/* Message is not shown */
MESSAGE
ttFoo.cField1
VIEW-AS ALERT-BOX
.
END.
Some comments, with unintended risk of hijacking the issue.
FIND FIRST ttFoo.
FOR EACH lbFoo WHERE
lbFoo.cField1 = "1":U AND
ttFoo.cField2 = lbFoo.cField2
Is valid for the compiler, but I prefer
FIND FIRST ttFoo.
FOR EACH lbFoo WHERE
lbFoo.cField1 = "1":U AND
lbFoo.cField2 = ttFoo.cField2
Maybe a rule to check if the "left hand side" of the condition is on a field of the, or a, buffer of the query. That rule would fire for your example and my first sample.
I think it would be great to detect these easily overlooked typos.
The rule should not fire for
FIND FIRST ttFoo.
FOR EACH lbFoo WHERE
can-do("a*", lbFoo.cField1) AND
(Field2Value = ? or lbFoo.cField2 = Field2Value)
can-douses a function to test a lbFoo field so it's OK(Field2Value = ? or lbFoo.cField2 = Field2Value)tests if the query value is known before using it.
Both are examples of code that may be totally misguided as far as performance (index usage) is concerned but to me, they would not be a violation of the rule.
I'm not sure i agree with your two examples. As you said, can-do should not be used in general for performance reasons. (It even has its own rule.) I also see no necessity for the second example, there are ways to to this in a clean way without the = ? check in the query, which will improve both performance and readability. I see no good reason to write queries like this.
Regarding the first example of @cverbiest, isn't it similar to @movedoa first example ? Issue is raised when the left-part of a simple comparison doesn't reference the buffer while the right-part does. That would raise an issue in both examples. Second example of @cverbiest wouldn't raise an issue.
Regarding the first example of @cverbiest, isn't it similar to @movedoa first example ? Issue is raised when the left-part of a simple comparison doesn't reference the buffer while the right-part does. That would raise an issue in both examples. Second example of @cverbiest wouldn't raise an issue.
Yes it is, and i'm happy with the rule when it only raises an issue for simple comparisons.
First version added in the develop branch
I know this is only a first version but just to inform you:
The rule currently does not work with CAN-FIND.
@movedoa Some improvements have been done on the rule, and so far can-find is handled correctly in the unit tests. Do you have simple examples so that I can add them to the tests ?
Will the improvements be in the hotfix? If yes i will look for false negatives after the update.
The improvements will be included in next version (which will probably be released tomorrow)