I completely agree with you, Mark. Though, I don't think it's
counter-intuitive for a procedure to change one of it's parameters --
not at all -- but I don't think this procedure (as written) is properly
encapsulated.
The problem is, it requires a global variable (filerec) and it requires
one of it's parameters to be located in an area of memory that's inside
that global parameter. That means that the caller has to know quite a
bit about how the procedure works -- and that's not a good thing. The
dependency is ugly and counter-intuitive. Furthermore, the caller has
to know what file is being used and how fields inside that file work.
That means you can't change this procedure to get it's information from
a different file, a different database, or some other means of storage
because the callers need to know about how that storage works, and
they'd all need to be changed.
On the other hand, if this is only used internally -- as part of a
larger routine -- and is never exported to other callers, it might be
worth using something like this as a means of saving programmer effort,
reducing debugging time, and simplifying maintenance in the long run.
A much better solution is to use dynamic SQL, as others have cited. But
the OP wasn't willing to do that.
You could accomplish the same thing that SQL does using record-level
I/O, but it'd be a lot of work. You'd need to create a service program
or something like that which uses APIs to get the from/to position of
the database fields, loads them into an array, so you can use %LOOKUP
(or something equivalent) to use the field name to get the record
positions, and do it that way. I know of at least two or three articles
that have demonstrated how to do exactly that -- but, it results in
quite a lot more code than this procedure did.
Another possibility is to simply pass an offset into the data structure.
I.e. instead of passing the actual field name or address in memory,
pass a starting record position and field length into the procedure, and
then use %subst() to view that part of the record. You could still use
pointer logic to determine where that starting position is, so you don't
have to hard-code it. This is nearly as simple as the original example,
but still requires the caller to know something about the record format,
so it's not as good as some of the other alternatives, but is simpler.
As for how long it took me to figure out a solution -- I don't really
think that's due to the complexity of the code (I can write code like
this in my sleep). It took so long because I couldn't figure out what
Don was looking for. That's just the way I am. I'm great at
understanding computers, but terrible at understanding people. I just
don't get people. They're not very intuitive.
But, I definitely agree with you, Mark.. this isn't a good example of
how to do things. I showed Don how to code exactly what he was
attempting to code without giving too much thought to whether it was a
good idea to do it or not.
Murphy, Mark wrote:
You know what? This is an elegant solution to a difficult problem.
And it will work well. But, six months down the road, it will be
unmaintainable to the vast majority of RPG programmers.
midrange.com
