Re: trying to be modular

On 12/8/2010 8:51 AM, David FOXWELL wrote:

This is an attempt at modularising my code. At the risk of getting slammed, I ask if it is acceptable and to tell me what the disadvantages may be and how I should have done this.

Had to get an authorization level from a user information table to condition a screen indicator in an existing program. The user table contains lots of other information like name, department and telephone number, etc.

I think many of us have gone down this path, but your post will be a
great reference for those who haven't yet. Thanks for posting!

In my pgm :

// *IN49 is already on.

IF GetAutLevel >= '4';
*IN49 = *OFF;

ENDIF;

As an outsider (or maybe a new programmer in your organisation?) I have
to ask what '4' might mean, and what indicator 49 does. Referring to
them by a named constant would make my comprehension a bit easier, so:
IF GetAutLevel >= CONFIDENTIAL;
HIDE_ID_NUMBER = *OFF;
ENDIF;

In a separate module where the user table is already used, I added :


P GetAutLevel B EXPORT
D PI LIKE ( DFN_AUTLV )
D UserId LIKE ( DFN_USRID ) CONST
D Error LIKE ( DFN_ERRMG )

D wAutLv S LIKE ( DFN_AUTLV )

IF NOT SetUserInfo ( UserId : Error );
RETURN *BLANK;

ENDIF;

RETURN UAUTNV; // From user table

P GetAutLevel E

I can't tell from just this bit of code what is being 'set' in the user
info. So I'll have to go open up that function to see what it does.
Apparently, it does something to (at least) UAUTNV.

P SetUserInfo B
D PI N
D UserId LIKE ( DFN_USRID ) CONST
D Error LIKE ( DFN_ERRMG )

D wUserId S LIKE ( DFN_USRID )
D STATIC

IF wUsrId <> UserId;
CHAIN UserId USRCFD;
If not %found;
error = ....
RETURN *OFF;

ENDIF;

RETURN *ON;

P SetUserInfo B

It doesn't look like a 'set' but a 'get'. And here's where the decision
making gets harder. There is no 'right' answer. Is there more user
info somewhere else, or is this all of it? Is there a structure that
holds the combined user info? Is it global? Should it be?

This returns an indicator and changes parameter 'Error.' The caller
tests the indicator but not Error. Is this duplicate functionality?
Could this return Error instead, and the caller test for NORMAL? Shop
standards could go either way, and I personally wouldn't have a
complaint with either one.

I like the pseudo-caching of the retrieved database record with the
static user ID. Overall I rather like this general design.
--buck