Want to change GetNode to Node

Thu, 2008-08-14 16:46 — jmls

I want to rename the ParseUnit GetNode method to simply Node, and NodeTypes GetNodeType to NodeType

 Message ParseUnit:GetNode(1):NodeText view-as alert-box.

would become

 Message ParseUnit:Node(1):NodeText view-as alert-box.

also, I would like to add Item as a synonym for Node / NodeType

 Message ParseUnit:Item(1):NodeText view-as alert-box.
 Message Proparse:NodeType:Item(1):TypeName view-as alert-box.

any objections to this ?

I want to differentiate between methods that return a value GetNodeText(nodenum) and an object Node(NodeNum)

Proparse discussion

Thu, 2008-08-14 17:15 — john

Re: Want to change GetNode to Node

That sounds good to me. A couple more trivial suggestions (I'm not fussy about these)...

message parseUnit:node(1):text view-as alert-box.

Use text rather than nodeText

Perhaps we should follow suite with other languages. Use UpperCase when referencing a Class, and lowerCase when referencing anything else (object instance, variable, property, method). You will find more and more people using a mix of OE+C#, or OE+Java, and this will be easier on their eyes. :)

Thu, 2008-08-14 17:25 — jmls

so, this would be message

so, this would be

message PARSEUNIT:NODE(1):nodetext view-as alert-box.

message ParseUnit:Node(1):nodetext view-as alert-box.

Thu, 2008-08-14 17:53 — john

No - it's just the first

No - it's just the first letter that is traditionally uppercase for a class name, like ClassName.cls.

def var parseUnit as class ParseUnit.
...
message parseUnit:node(1):txt view-as alert-box.

Coming from Java (or C#), when I glance at this:

message ParseUnit:Node(1):nodetext view-as alert-box.

I get confused because ParseUnit suggests to me that you are refering to the class, which would mean to me that node() would have to be a static class method.

Thu, 2008-08-14 17:23 — jmls

I did want to do that

I did want to do that originally, but progress complains with a syntax error

 DEF PUBLIC PROPERTY Text     AS CHAR NO-UNDO GET .  SET .

does not compile. Neither does "Col"

So, once text was not available, it didn't make sense to have Node:Index, Node:Line, but Node:NodeText and Node:NodeCol so I settled on Node:NodeLine etc for consistency

Thu, 2008-08-14 17:41 — john

Sheesh. Stupid reserved

Sheesh. Stupid reserved keywords!

It seems a shame to make all of the property names so long just because a few of the names we want are reserved. node:nodeLine is an awful lot of clutter. It's as bad as using 'getters' for everything! Like node:getLine(). Properties in ABL are a great language feature, and using 'node' in every property name almost defeats the purpose.

How about non-keyword names, like 'txt' and 'colmn'? We don't really have to remember those goofy names, because we have code-completion in OE Architect.

Thu, 2008-08-14 17:50 — jmls

so, the properties of class

so, the properties of class node become

nodetext == txt
nodecol == colm
nodeindex == idx
nodeline == line
nodefilenum == filenum
nodetypenum == typenum

NodeType == Type (class NodeType)

I *dont* like colm, but can't think of a better name ...

Thu, 2008-08-14 23:29 — jurjen

"colm" sure is an ugly

"colm" sure is an ugly name.
Generally I like to avoid abbreviations, so I would consider (apart from casing):

colm = columnposition
line = linenumber
filenum = fileindex
typenum = typenumber (or is it actually typeindex)

Thu, 2008-08-14 23:45 — jurjen

NumNodes = Count ?

just to split some hairs... "ParseUnit:NumNodes" reminds me of "Nodes:Length" (when in Java) or "Nodes:Count" in some other languages.

a collection class usually has a Length or Count property.
ParseUnit is not really a collection of Nodes, but close enough, and since you also brought up Item(n) I figured it might make sense to also have a Count property.

Fri, 2008-08-15 06:47 — jmls

it would be a trivial task

it would be a trivial task to add a nodes collection to the parseunit (it would actually be a lot "neater" in design)

so instead of

 Message parseUnit:Node(1):Txt view-as alert-box.

we would have

 Message parseUnit:Nodes:Item(1):Txt view-as alert-box.
 Message parseUnit:Nodes:Count view-as alert-box.

Would that be better ?

Fri, 2008-08-15 18:19 — john

Re: parseUnit:Nodes

Wouldn't it make the implementation more clumsy? Aren't there lazy-load methods that operate on the blob that would have to be shared between ParseUnit and a new collection class, say, 'NodeList'?

It just sounds like extra work to me. I don't know the state of Collection classes in ABL 10.1C. I suppose it could work and be sensible if NodeList were a subclass of a standard Collection class, and NodeList was free to implement all its methods as lazy-load methods operating on the blob.

I'd find this sort of thing easy to understand and use:

def var nodeList as class NodeList.
def var node as class Node.
nodeList = parseUnit:nodeList.
do i = 1 to nodeList:size:
  node = nodeList:get(i).
  ...
end.

Operating on the list of nodes is not really a normal way of doing things anyway. Getting parseUnit:topNode and then walking down through the syntax tree is common. Getting a list of nodes with something like parseUnit:queryType("DEFINE") is very common in Prolint.

Fri, 2008-08-15 19:05 — jmls

you could do that, and also

you could do that, and also do

def var node as class Node.
.
do i = 1 to parseUnit:nodeList:size:
  node = parseUnit:nodeList:get(i).
  ...
end.

there is nothing new in collection classes for 10.1C. Unfortunately.

Fri, 2008-08-15 19:36 — john

I would use the "introduce

I would use the "introduce variable" refactoring on that particular piece of code.

I don't know about ABL, but in most other languages there is some overhead in the use of an accessor method. (nodeList is a PROPERTY, right?) I would introduce a variable even if I was only using the accessor a few times. In a tight loop like this, reducing function calls is almost always best practice.

(Even if ABL does something clever so there's no overhead in a default GET, that doesn't mean that the author of the GET method won't change its implementation the future.)

That raises a question about the implementation of Schema.cls. Is there a difference in performance between using a PROPERTY and using a public var? A PROPERTY (with accessors) is always better practice, but Schema.cls is not really intended to be a 'public' part of the API, and performance trumps best practice.

Fri, 2008-08-15 19:50 — jmls

I don't know about ABL, but

I don't know about ABL, but in most other languages there is some overhead in the use of an accessor method. (nodeList is a PROPERTY, right?) I would introduce a variable even if I was only using the accessor a few times. In a tight loop like this, reducing function calls is almost always best practice.

Absolutely, that's what I would do. However, I was just noting that it could be done this way.

That raises a question about the implementation of Schema.cls. Is there a difference in performance between using a PROPERTY and using a public var? A PROPERTY (with accessors) is always better practice, but Schema.cls is not really intended to be a 'public' part of the API, and performance trumps best practice.

the following code:

CLASS test   : 

 DEF PUBLIC VAR foo AS CHAR INIT "foo" NO-UNDO.
 DEF PUBLIC PROPERTY bar AS CHAR INIT "bar" NO-UNDO GET . SET .

END CLASS.

def var a as class test no-undo.

def var i as int no-undo.
def var s as int no-undo.
def var b as char no-undo.

a = new test().

s = mtime.

do i = 1 to 1000000:
 b = a:foo.
end.

message mtime - s view-as alert-box.


s = mtime.

do i = 1 to 1000000:
 b = a:bar.
end.

message mtime - s view-as alert-box.


delete object a.

gives the following results:

accessing a:foo 809ms
accessing a:bar 800ms

so a property is actually faster :)

Fri, 2008-08-15 20:07 — john

The OpenEdge Hive

More Navigation