A small rant on the lack of comments...

Look at this: Guys, what's the point of doing any refactoring if the code is going to be as inscrutable as before? MAKEUS^SDAMAPI2(RETURN,DFN,SC,SD,TYP,STYP,CIO) ; Make unscheduled appointment N SCAP,STAT,%,TYPE,S,CLN,SM K RETURN S RETURN=0 S:$D(TYP) TYPE=+TYP S:$G(SD)="" SD=$$NOW^XLFDT() I +$G(SD)=0!'($G(SD)#1) S RETURN=0,TXT(1)="SD" D ERRX^SDAPIE(.RETURN,"INVPARAM",.TXT) Q 0 S %=$$CHKAPTU(.RETURN,.SC,.DFN,.SD,,,1) Q:RETURN=0 0 S %=$$CHKTYPE^SDMAPI5(.RETURN,+DFN,.TYP) Q:'% 0 S %=$$CHKSTYP^SDMAPI5(.RETURN,$G(TYP),.STYP) Q:'% 0 I +SD>$$FMADD^XLFDT(DT,1) S RETURN=0 D ERRX^SDAPIE(.RETURN,"INVPARAM","SD") Q 0 S SD=$J(SD,2,4) S STAT=$$INP^SDAM2(+DFN,+SD) D GETCLN^SDMDAL1(.CLN,+SC,1) D MAKE^SDMDAL3(+DFN,+SD,+SC,+TYPE,+$G(STYP),STAT,4,DUZ,DT,"W",0) D MAKE^SDMDAL1(+SC,+SD,+DFN,CLN(1912),,DUZ) S %=$$LOCKST^SDMDAL1(+SC,+SD) I '% S RETURN=0 D ERRX^SDAPIE(.RETURN,"APTLOCK") Q 0 S SM=$$DECAVA^SDMAPI3(.CLN,+SC,+SD,CLN(1912),.S) D SETST^SDMDAL1(+SC,+SD,S) D UNLCKST^SDMDAL1(+SC,+SD) What's worse, this is an API! You don't tell me what the parameters stand for. I won't point to my code for examples, but take a look at this random well commented routine: MAGGSIV. Sam
like0

Comments

There is an API document

Afsin Ustundag's picture

Sam, there is an API document in https://github.com/kthlnkeating/VistA-FOIA/tree/master/Packages/Scheduling/doc which is also included in the OSEHRA technical journal.  There is a quite a bit of information in there for each API entry and each parameter and some of them are a few pages long so it was not practical to keep a version in the code as well.  We did discuss putting all the information in the code and using/developing a Javadoc like tool for API documentation but we never got there.

For in code documentation in my programming career I've seen in code documention that were not updated after changes, I've seen in code documentation that were wrong and confused matters more, I 've seen in code documentation that was not useful at all.  What matters most is not in code documentation but to write code that is easily followable.  Now if you are saying that you cannot follow the API code easily then that is a problem we should correct and any suggestion is welcome.  But I will always resist to put comments of the following sort

 ; Do we have any data ?
 I ($D(MAGARRAY)<10) S MAGRY(0)="0^No input data, Operation CANCELED" Q
 

 

.

 

 

 

like0

A small rant on the lack of comments...

Roy Gaber's picture

This has long been an item of contention within VA. One of the reasons
for the lack of variable definitions, in my opinion, if that back when
VistA (DHCP) was developed the VA was working with what technology was
available to it at the time. Since M(UMPS) is an interpreted language
adding comments to code would slow execution because the interpreter
needed to evaluate the comment line anyway, the thought was that the
less comments the faster the execution. At the time there was limited
amount of RAM which could be put into the initial systems DHCP was
developed on. Even though technology grew at an accelerated rate, I
believe the practice of no comments stuck and just became the norm.

I have been developing software for the VA for quite a number of years
and it has become standard practice for me to trace through the code to
determine what the passed in variables were supposed to be, not that
this is a practice I condone, but what other choice did I have.

I think that part of the OSEHRA focus could be on people such as
yourself making the comments in the code for possible inclusion of the
"gold" code.

Just some random thoughts.

-----Original Message-----
From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Sam Habiel
Sent: Friday, May 03, 2013 6:23 PM
To: ehr-refactoring-services@groups.osehra.org
Subject: [ehr-refactoring-services] A small rant on the lack of
comments...

Look at this:

Guys, what's the point of doing any refactoring if the code is going to
be as inscrutable as before?

MAKEUS^SDAMAPI2(RETURN,DFN,SC,SD,TYP,STYP,CIO) ; Make unscheduled
appointment
N SCAP,STAT,%,TYPE,S,CLN,SM
K RETURN S RETURN=0
S:$D(TYP) TYPE=+TYP S:$G(SD)="" SD=$$NOW^XLFDT()
I +$G(SD)=0!'($G(SD)#1) S RETURN=0,TXT(1)="SD" D
ERRX^SDAPIE(.RETURN,"INVPARAM",.TXT) Q 0
S %=$$CHKAPTU(.RETURN,.SC,.DFN,.SD,,,1) Q:RETURN=0 0
S %=$$CHKTYPE^SDMAPI5(.RETURN,+DFN,.TYP) Q:'% 0
S %=$$CHKSTYP^SDMAPI5(.RETURN,$G(TYP),.STYP) Q:'% 0
I +SD>$$FMADD^XLFDT(DT,1) S RETURN=0 D
ERRX^SDAPIE(.RETURN,"INVPARAM","SD") Q 0
S SD=$J(SD,2,4)
S STAT=$$INP^SDAM2(+DFN,+SD)
D GETCLN^SDMDAL1(.CLN,+SC,1)
D MAKE^SDMDAL3(+DFN,+SD,+SC,+TYPE,+$G(STYP),STAT,4,DUZ,DT,"W",0)
D MAKE^SDMDAL1(+SC,+SD,+DFN,CLN(1912),,DUZ)
S %=$$LOCKST^SDMDAL1(+SC,+SD) I '% S RETURN=0 D
ERRX^SDAPIE(.RETURN,"APTLOCK") Q 0
S SM=$$DECAVA^SDMAPI3(.CLN,+SC,+SD,CLN(1912),.S)
D SETST^SDMDAL1(+SC,+SD,S)
D UNLCKST^SDMDAL1(+SC,+SD)

What's worse, this is an API! You don't tell me what the parameters
stand for.

I won't point to my code for examples, but take a look at this random
well commented routine: MAGGSIV.

Sam

like0

A small rant on the lack of comments...

Wes Turner's picture

I asked Joe to check the SAC, and here is his response ...

According to http://www.hardhats.org/tools/sac07.html

There are only a few entries that really seem to apply to comments:

2.2.7 The maximum routine size, as determined by executing
^%ZOSF("SIZE"), is 20,000 characters. 15,000 of the allowed 20,000
characters may be non-comment source lines (including “;;” comment lines).
5,000 characters are reserved for non-“;;” comments.

2.10.4.1 Routine line tags referenced from outside the routine should
include a comment describing the code’s function after theline tag.

2.10.4.2 Any supported references or routines invoked initially from an
option or protocol should contain comments explaining the functionality and
all input and output variables.

Basically, I would claim you are both correct. More comments would be nice,
but they are not required by the SAC. I think the solution Afsin alludes to
would be a big improvement ... Code a document generator that can turn
comments into documents. Then there is a mechanism for review and
correction of comments, and the documentation, comments and code stay in
sync. This is the mechanism used by other open source projects, such as
e.g. ITK (http://www.itk.org/Doxygen/html/index.html).

- Wes

On Mon, May 6, 2013 at 9:28 AM, "Gaber, Roy G." <Roy.Gaber@va.gov> wrote:

> This has long been an item of contention within VA. One of the reasons
> for the lack of variable definitions, in my opinion, if that back when
> VistA (DHCP) was developed the VA was working with what technology was
> available to it at the time. Since M(UMPS) is an interpreted language
> adding comments to code would slow execution because the interpreter
> needed to evaluate the comment line anyway, the thought was that the
> less comments the faster the execution. At the time there was limited
> amount of RAM which could be put into the initial systems DHCP was
> developed on. Even though technology grew at an accelerated rate, I
> believe the practice of no comments stuck and just became the norm.
>
> I have been developing software for the VA for quite a number of years
> and it has become standard practice for me to trace through the code to
> determine what the passed in variables were supposed to be, not that
> this is a practice I condone, but what other choice did I have.
>
> I think that part of the OSEHRA focus could be on people such as
> yourself making the comments in the code for possible inclusion of the
> "gold" code.
>
> Just some random thoughts.
>
> -----Original Message-----
> From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Sam Habiel
> Sent: Friday, May 03, 2013 6:23 PM
> To: ehr-refactoring-services@groups.osehra.org
> Subject: [ehr-refactoring-services] A small rant on the lack of
> comments...
>
> Look at this:
>
> Guys, what's the point of doing any refactoring if the code is going to
> be as inscrutable as before?
>
> MAKEUS^SDAMAPI2(RETURN,DFN,SC,SD,TYP,STYP,CIO) ; Make unscheduled
> appointment
> N SCAP,STAT,%,TYPE,S,CLN,SM
> K RETURN S RETURN=0
> S:$D(TYP) TYPE=+TYP S:$G(SD)="" SD=$$NOW^XLFDT()
> I +$G(SD)=0!'($G(SD)#1) S RETURN=0,TXT(1)="SD" D
> ERRX^SDAPIE(.RETURN,"INVPARAM",.TXT) Q 0
> S %=$$CHKAPTU(.RETURN,.SC,.DFN,.SD,,,1) Q:RETURN=0 0
> S %=$$CHKTYPE^SDMAPI5(.RETURN,+DFN,.TYP) Q:'% 0
> S %=$$CHKSTYP^SDMAPI5(.RETURN,$G(TYP),.STYP) Q:'% 0
> I +SD>$$FMADD^XLFDT(DT,1) S RETURN=0 D
> ERRX^SDAPIE(.RETURN,"INVPARAM","SD") Q 0
> S SD=$J(SD,2,4)
> S STAT=$$INP^SDAM2(+DFN,+SD)
> D GETCLN^SDMDAL1(.CLN,+SC,1)
> D MAKE^SDMDAL3(+DFN,+SD,+SC,+TYPE,+$G(STYP),STAT,4,DUZ,DT,"W",0)
> D MAKE^SDMDAL1(+SC,+SD,+DFN,CLN(1912),,DUZ)
> S %=$$LOCKST^SDMDAL1(+SC,+SD) I '% S RETURN=0 D
> ERRX^SDAPIE(.RETURN,"APTLOCK") Q 0
> S SM=$$DECAVA^SDMAPI3(.CLN,+SC,+SD,CLN(1912),.S)
> D SETST^SDMDAL1(+SC,+SD,S)
> D UNLCKST^SDMDAL1(+SC,+SD)
>
> What's worse, this is an API! You don't tell me what the parameters
> stand for.
>
> I won't point to my code for examples, but take a look at this random
> well commented routine: MAGGSIV.
>
> Sam
>
>
>
>
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions:
> http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post:
> http://www.osehra.org/og_mailinglist/unsubscribe/1846
>
>

--
Wesley D. Turner, Ph.D.
Kitware, Inc.
Technical Leader
28 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4920

like0

A small rant on the lack of comments...

Roy Gaber's picture

The SACC really ought to revise their standards given that the open
source community is used to documenting their API's/code. The document
generator is a good idea and should be easily implemented in M(UMPS).

From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Wes Turner
Sent: Monday, May 06, 2013 11:51 AM
To: ehr-refactoring-services@groups.osehra.org
Subject: Re: [ehr-refactoring-services] A small rant on the lack of
comments...

I asked Joe to check the SAC, and here is his response ...

According to http://www.hardhats.org/tools/sac07.html
<http://www.hardhats.org/tools/sac07.html>

There are only a few entries that really seem to apply to comments:

2.2.7 The maximum routine size, as determined by executing
^%ZOSF("SIZE"), is 20,000 characters. 15,000 of the allowed 20,000
characters may be non-comment source lines (including ";;" comment
lines). 5,000 characters are reserved for non-";;" comments.

2.10.4.1 Routine line tags referenced from outside the routine should
include a comment describing the code's function after theline tag.

2.10.4.2 Any supported references or routines invoked initially from
an option or protocol should contain comments explaining the
functionality and all input and output variables.

Basically, I would claim you are both correct. More comments would be
nice, but they are not required by the SAC. I think the solution Afsin
alludes to would be a big improvement ... Code a document generator that
can turn comments into documents. Then there is a mechanism for review
and correction of comments, and the documentation, comments and code
stay in sync. This is the mechanism used by other open source projects,
such as e.g. ITK (http://www.itk.org/Doxygen/html/index.html).

- Wes

On Mon, May 6, 2013 at 9:28 AM, "Gaber, Roy G." <Roy.Gaber@va.gov>
wrote:

This has long been an item of contention within VA. One of the reasons
for the lack of variable definitions, in my opinion, if that back when
VistA (DHCP) was developed the VA was working with what technology was
available to it at the time. Since M(UMPS) is an interpreted language
adding comments to code would slow execution because the interpreter
needed to evaluate the comment line anyway, the thought was that the
less comments the faster the execution. At the time there was limited
amount of RAM which could be put into the initial systems DHCP was
developed on. Even though technology grew at an accelerated rate, I
believe the practice of no comments stuck and just became the norm.

I have been developing software for the VA for quite a number of years
and it has become standard practice for me to trace through the code to
determine what the passed in variables were supposed to be, not that
this is a practice I condone, but what other choice did I have.

I think that part of the OSEHRA focus could be on people such as
yourself making the comments in the code for possible inclusion of the
"gold" code.

Just some random thoughts.

-----Original Message-----
From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Sam Habiel
Sent: Friday, May 03, 2013 6:23 PM
To: ehr-refactoring-services@groups.osehra.org
Subject: [ehr-refactoring-services] A small rant on the lack of
comments...

Look at this:

Guys, what's the point of doing any refactoring if the code is going to
be as inscrutable as before?

MAKEUS^SDAMAPI2(RETURN,DFN,SC,SD,TYP,STYP,CIO) ; Make unscheduled
appointment
N SCAP,STAT,%,TYPE,S,CLN,SM
K RETURN S RETURN=0
S:$D(TYP) TYPE=+TYP S:$G(SD)="" SD=$$NOW^XLFDT()
I +$G(SD)=0!'($G(SD)#1) S RETURN=0,TXT(1)="SD" D
ERRX^SDAPIE(.RETURN,"INVPARAM",.TXT) Q 0
S %=$$CHKAPTU(.RETURN,.SC,.DFN,.SD,,,1) Q:RETURN=0 0
S %=$$CHKTYPE^SDMAPI5(.RETURN,+DFN,.TYP) Q:'% 0
S %=$$CHKSTYP^SDMAPI5(.RETURN,$G(TYP),.STYP) Q:'% 0
I +SD>$$FMADD^XLFDT(DT,1) S RETURN=0 D
ERRX^SDAPIE(.RETURN,"INVPARAM","SD") Q 0
S SD=$J(SD,2,4)
S STAT=$$INP^SDAM2(+DFN,+SD)
D GETCLN^SDMDAL1(.CLN,+SC,1)
D MAKE^SDMDAL3(+DFN,+SD,+SC,+TYPE,+$G(STYP),STAT,4,DUZ,DT,"W",0)
D MAKE^SDMDAL1(+SC,+SD,+DFN,CLN(1912),,DUZ)
S %=$$LOCKST^SDMDAL1(+SC,+SD) I '% S RETURN=0 D
ERRX^SDAPIE(.RETURN,"APTLOCK") Q 0
S SM=$$DECAVA^SDMAPI3(.CLN,+SC,+SD,CLN(1912),.S)
D SETST^SDMDAL1(+SC,+SD,S)
D UNLCKST^SDMDAL1(+SC,+SD)

What's worse, this is an API! You don't tell me what the parameters
stand for.

I won't point to my code for examples, but take a look at this random
well commented routine: MAGGSIV.

Sam

--
Full post: http://www.osehra.org/content/small-rant-lack-comments
Manage my subscriptions:
http://www.osehra.org/og_mailinglist/subscriptions
Stop emails for this post:
http://www.osehra.org/og_mailinglist/unsubscribe/1846

--
Wesley D. Turner, Ph.D.
Kitware, Inc.
Technical Leader
28 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4920

like0

A small rant on the lack of comments...

Wes Turner's picture

Roy,

+1. We would need to develop a recommendation for the SAC change. Any
thoughts on what you would like to see it say?

- Wes

On Mon, May 6, 2013 at 11:54 AM, "Gaber, Roy G." <Roy.Gaber@va.gov> wrote:

> The SACC really ought to revise their standards given that the open source
> community is used to documenting their API’s/code. The document generator
> is a good idea and should be easily implemented in M(UMPS). ****
>
> ** **
>
> *From:* Apache [mailto:apache@groups.osehra.org] *On Behalf Of *Wes Turner
>
> *Sent:* Monday, May 06, 2013 11:51 AM
> *To:* ehr-refactoring-services@groups.osehra.org
> *Subject:* Re: [ehr-refactoring-services] A small rant on the lack of
> comments...****
>
> ** **
>
> I asked Joe to check the SAC, and here is his response ...****
>
> ** **
>
> According to http://www.hardhats.org/tools/sac07.html
>
> There are only a few entries that really seem to apply to comments:****
>
> 2.2.7 The maximum routine size, as determined by executing
> ^%ZOSF("SIZE"), is 20,000 characters. 15,000 of the allowed 20,000
> characters may be non-comment source lines (including “;;” comment lines).
> 5,000 characters are reserved for non-“;;” comments.****
>
> 2.10.4.1 Routine line tags referenced from outside the routine should
> include a comment describing the code’s function after theline tag.****
>
> 2.10.4.2 Any supported references or routines invoked initially from an
> option or protocol should contain comments explaining the functionality and
> all input and output variables.****
>
> Basically, I would claim you are both correct. More comments would be
> nice, but they are not required by the SAC. I think the solution Afsin
> alludes to would be a big improvement ... Code a document generator that
> can turn comments into documents. Then there is a mechanism for review and
> correction of comments, and the documentation, comments and code stay in
> sync. This is the mechanism used by other open source projects, such as
> e.g. ITK (http://www.itk.org/Doxygen/html/index.html).****
>
> ** **
>
> - Wes****
>
> ** **
>
> On Mon, May 6, 2013 at 9:28 AM, "Gaber, Roy G." <Roy.Gaber@va.gov> wrote:*
> ***
>
> This has long been an item of contention within VA. One of the reasons
> for the lack of variable definitions, in my opinion, if that back when
> VistA (DHCP) was developed the VA was working with what technology was
> available to it at the time. Since M(UMPS) is an interpreted language
> adding comments to code would slow execution because the interpreter
> needed to evaluate the comment line anyway, the thought was that the
> less comments the faster the execution. At the time there was limited
> amount of RAM which could be put into the initial systems DHCP was
> developed on. Even though technology grew at an accelerated rate, I
> believe the practice of no comments stuck and just became the norm.
>
> I have been developing software for the VA for quite a number of years
> and it has become standard practice for me to trace through the code to
> determine what the passed in variables were supposed to be, not that
> this is a practice I condone, but what other choice did I have.
>
> I think that part of the OSEHRA focus could be on people such as
> yourself making the comments in the code for possible inclusion of the
> "gold" code.
>
> Just some random thoughts.****
>
>
> -----Original Message-----
> From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Sam Habiel
> Sent: Friday, May 03, 2013 6:23 PM
> To: ehr-refactoring-services@groups.osehra.org
> Subject: [ehr-refactoring-services] A small rant on the lack of
> comments...
>
> Look at this:
>
> Guys, what's the point of doing any refactoring if the code is going to
> be as inscrutable as before?
>
> MAKEUS^SDAMAPI2(RETURN,DFN,SC,SD,TYP,STYP,CIO) ; Make unscheduled
> appointment
> N SCAP,STAT,%,TYPE,S,CLN,SM
> K RETURN S RETURN=0
> S:$D(TYP) TYPE=+TYP S:$G(SD)="" SD=$$NOW^XLFDT()
> I +$G(SD)=0!'($G(SD)#1) S RETURN=0,TXT(1)="SD" D
> ERRX^SDAPIE(.RETURN,"INVPARAM",.TXT) Q 0
> S %=$$CHKAPTU(.RETURN,.SC,.DFN,.SD,,,1) Q:RETURN=0 0
> S %=$$CHKTYPE^SDMAPI5(.RETURN,+DFN,.TYP) Q:'% 0
> S %=$$CHKSTYP^SDMAPI5(.RETURN,$G(TYP),.STYP) Q:'% 0
> I +SD>$$FMADD^XLFDT(DT,1) S RETURN=0 D
> ERRX^SDAPIE(.RETURN,"INVPARAM","SD") Q 0
> S SD=$J(SD,2,4)
> S STAT=$$INP^SDAM2(+DFN,+SD)
> D GETCLN^SDMDAL1(.CLN,+SC,1)
> D MAKE^SDMDAL3(+DFN,+SD,+SC,+TYPE,+$G(STYP),STAT,4,DUZ,DT,"W",0)
> D MAKE^SDMDAL1(+SC,+SD,+DFN,CLN(1912),,DUZ)
> S %=$$LOCKST^SDMDAL1(+SC,+SD) I '% S RETURN=0 D
> ERRX^SDAPIE(.RETURN,"APTLOCK") Q 0
> S SM=$$DECAVA^SDMAPI3(.CLN,+SC,+SD,CLN(1912),.S)
> D SETST^SDMDAL1(+SC,+SD,S)
> D UNLCKST^SDMDAL1(+SC,+SD)
>
> What's worse, this is an API! You don't tell me what the parameters
> stand for.
>
> I won't point to my code for examples, but take a look at this random
> well commented routine: MAGGSIV.
>
> Sam
>
>
>
> ****
>
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions:
> http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post:
> http://www.osehra.org/og_mailinglist/unsubscribe/1846****
>
>
>
> ****
>
> ** **
>
> --
> Wesley D. Turner, Ph.D.
> Kitware, Inc.
> Technical Leader
> 28 Corporate Drive
> Clifton Park, NY 12065-8662
> Phone: 518-881-4920 ****
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions:
> http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post:
> http://www.osehra.org/og_mailinglist/unsubscribe/1846
>
>

--
Wesley D. Turner, Ph.D.
Kitware, Inc.
Technical Leader
28 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4920

like0

SAC

Robert Sax's picture

So why would we need to change the SAC? Can't OSEHRA build it's own "SAC" starting with the VA's and add to it as desired? 

like0

A small rant on the lack of comments...

Roy Gaber's picture

OSEHRA is the VA’s

From: Apache [mailto:apache@groups.osehra.org] On Behalf Of rsax
Sent: Monday, May 06, 2013 12:06 PM
To: EHR Refactoring Services
Subject: Re: [ehr-refactoring-services] A small rant on the lack of comments...

So why would we need to change the SAC? Can't OSEHRA build it's own "SAC" starting with the VA's and add to it as desired?

--
Full post: http://www.osehra.org/content/small-rant-lack-comments
Manage my subscriptions: http://www.osehra.org/og_mailinglist/subscriptions
Stop emails for this post: http://www.osehra.org/og_mailinglist/unsubscribe/1846

like0

A small rant on the lack of comments...

Wes Turner's picture

OSEHRA uses the VA SAC as their initial set of standards because of
convenience, because it largely matches what the code already has, and
because there are forking implications with having incompatible SACs. That
said, it is a set of community norms and we can change it if there is
sufficient community support. We can also provide suggestions back to the
VA; although, the VA is a separate entity and we cannot guarantee that the
VA will take the suggestions.

As Roy notes, changing the SAC has implications and we need to be careful
that we either grandfather old code in, or have a workable plan for change;
and we need to be prepared for the VA to disagree. I would not look
forward to taking every VA supplied patch, and modifying it after the fact
to conform to different standards before taking it into the OSEHRA code
base!

That said, why not start the conversation. What specific language you would
like to see in the SAC?

- Wes

On Mon, May 6, 2013 at 12:07 PM, "Gaber, Roy G." <Roy.Gaber@va.gov> wrote:

> OSEHRA is the VA’s****
>
> ** **
>
> *From:* Apache [mailto:apache@groups.osehra.org] *On Behalf Of *rsax
> *Sent:* Monday, May 06, 2013 12:06 PM
> *To:* EHR Refactoring Services
>
> *Subject:* Re: [ehr-refactoring-services] A small rant on the lack of
> comments...****
>
> ** **
>
> So why would we need to change the SAC? Can't OSEHRA build it's own "SAC"
> starting with the VA's and add to it as desired? ****
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions:
> http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post:
> http://www.osehra.org/og_mailinglist/unsubscribe/1846****
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions:
> http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post:
> http://www.osehra.org/og_mailinglist/unsubscribe/1846
>
>

--
Wesley D. Turner, Ph.D.
Kitware, Inc.
Technical Leader
28 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4920

like0

A small rant on the lack of comments...

Roy Gaber's picture

Given that VA has paid for the OSEHRA project, that could be used for
the impetus for changing the standard, however, that will only address
new code, the existing codebase would have to be reviewed with
documentation being added as needed. This is a monumental task, but if
coordinated properly, and with enough volunteers it may be beneficial.

The process could be started by identifying the most needed
documentation and proceed from there.

From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Wes Turner
Sent: Monday, May 06, 2013 11:58 AM
To: ehr-refactoring-services@groups.osehra.org
Subject: Re: [ehr-refactoring-services] A small rant on the lack of
comments...

Roy,

+1. We would need to develop a recommendation for the SAC change. Any
thoughts on what you would like to see it say?

- Wes

On Mon, May 6, 2013 at 11:54 AM, "Gaber, Roy G." <Roy.Gaber@va.gov>
wrote:

The SACC really ought to revise their standards given that the open
source community is used to documenting their API's/code. The document
generator is a good idea and should be easily implemented in M(UMPS).

From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Wes Turner

Sent: Monday, May 06, 2013 11:51 AM
To: ehr-refactoring-services@groups.osehra.org

Subject: Re: [ehr-refactoring-services] A small rant on the lack of
comments...

I asked Joe to check the SAC, and here is his response ...

According to http://www.hardhats.org/tools/sac07.html
<http://www.hardhats.org/tools/sac07.html>

There are only a few entries that really seem to apply to comments:

2.2.7 The maximum routine size, as determined by executing
^%ZOSF("SIZE"), is 20,000 characters. 15,000 of the allowed 20,000
characters may be non-comment source lines (including ";;" comment
lines). 5,000 characters are reserved for non-";;" comments.

2.10.4.1 Routine line tags referenced from outside the routine should
include a comment describing the code's function after theline tag.

2.10.4.2 Any supported references or routines invoked initially from
an option or protocol should contain comments explaining the
functionality and all input and output variables.

Basically, I would claim you are both correct. More comments would be
nice, but they are not required by the SAC. I think the solution Afsin
alludes to would be a big improvement ... Code a document generator that
can turn comments into documents. Then there is a mechanism for review
and correction of comments, and the documentation, comments and code
stay in sync. This is the mechanism used by other open source projects,
such as e.g. ITK (http://www.itk.org/Doxygen/html/index.html).

- Wes

On Mon, May 6, 2013 at 9:28 AM, "Gaber, Roy G." <Roy.Gaber@va.gov>
wrote:

This has long been an item of contention within VA. One of the reasons
for the lack of variable definitions, in my opinion, if that back when
VistA (DHCP) was developed the VA was working with what technology was
available to it at the time. Since M(UMPS) is an interpreted language
adding comments to code would slow execution because the interpreter
needed to evaluate the comment line anyway, the thought was that the
less comments the faster the execution. At the time there was limited
amount of RAM which could be put into the initial systems DHCP was
developed on. Even though technology grew at an accelerated rate, I
believe the practice of no comments stuck and just became the norm.

I have been developing software for the VA for quite a number of years
and it has become standard practice for me to trace through the code to
determine what the passed in variables were supposed to be, not that
this is a practice I condone, but what other choice did I have.

I think that part of the OSEHRA focus could be on people such as
yourself making the comments in the code for possible inclusion of the
"gold" code.

Just some random thoughts.

-----Original Message-----
From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Sam Habiel
Sent: Friday, May 03, 2013 6:23 PM
To: ehr-refactoring-services@groups.osehra.org
Subject: [ehr-refactoring-services] A small rant on the lack of
comments...

Look at this:

Guys, what's the point of doing any refactoring if the code is going to
be as inscrutable as before?

MAKEUS^SDAMAPI2(RETURN,DFN,SC,SD,TYP,STYP,CIO) ; Make unscheduled
appointment
N SCAP,STAT,%,TYPE,S,CLN,SM
K RETURN S RETURN=0
S:$D(TYP) TYPE=+TYP S:$G(SD)="" SD=$$NOW^XLFDT()
I +$G(SD)=0!'($G(SD)#1) S RETURN=0,TXT(1)="SD" D
ERRX^SDAPIE(.RETURN,"INVPARAM",.TXT) Q 0
S %=$$CHKAPTU(.RETURN,.SC,.DFN,.SD,,,1) Q:RETURN=0 0
S %=$$CHKTYPE^SDMAPI5(.RETURN,+DFN,.TYP) Q:'% 0
S %=$$CHKSTYP^SDMAPI5(.RETURN,$G(TYP),.STYP) Q:'% 0
I +SD>$$FMADD^XLFDT(DT,1) S RETURN=0 D
ERRX^SDAPIE(.RETURN,"INVPARAM","SD") Q 0
S SD=$J(SD,2,4)
S STAT=$$INP^SDAM2(+DFN,+SD)
D GETCLN^SDMDAL1(.CLN,+SC,1)
D MAKE^SDMDAL3(+DFN,+SD,+SC,+TYPE,+$G(STYP),STAT,4,DUZ,DT,"W",0)
D MAKE^SDMDAL1(+SC,+SD,+DFN,CLN(1912),,DUZ)
S %=$$LOCKST^SDMDAL1(+SC,+SD) I '% S RETURN=0 D
ERRX^SDAPIE(.RETURN,"APTLOCK") Q 0
S SM=$$DECAVA^SDMAPI3(.CLN,+SC,+SD,CLN(1912),.S)
D SETST^SDMDAL1(+SC,+SD,S)
D UNLCKST^SDMDAL1(+SC,+SD)

What's worse, this is an API! You don't tell me what the parameters
stand for.

I won't point to my code for examples, but take a look at this random
well commented routine: MAGGSIV.

Sam

--
Full post: http://www.osehra.org/content/small-rant-lack-comments
Manage my subscriptions:
http://www.osehra.org/og_mailinglist/subscriptions
Stop emails for this post:
http://www.osehra.org/og_mailinglist/unsubscribe/1846

--
Wesley D. Turner, Ph.D.
Kitware, Inc.
Technical Leader
28 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4920

--
Full post: http://www.osehra.org/content/small-rant-lack-comments
Manage my subscriptions:
http://www.osehra.org/og_mailinglist/subscriptions
Stop emails for this post:
http://www.osehra.org/og_mailinglist/unsubscribe/1846

--
Wesley D. Turner, Ph.D.
Kitware, Inc.
Technical Leader
28 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4920

like0

This is the critical piece

Christopher Edwards's picture

I believe this is the critical section of the SAC that Sam is referring to:

2.10.4.2 Any supported references or routines invoked initially from an option or protocol should contain comments explaining the functionality and all input and output variables.

There is a debate as to wether documentation should live with the code or be in separate documentation. I tende to lean on the side of putting the documentation as close to the intended audience as possible.

In this case I'd vote (if it matters :) ) for a comment describing the Input and Output variables for the API as these will be used by developers. If separate documentation is needed it could be generated by doxoygen, docutils, etc.

like0

A small rant on the lack of comments...

Sam Habiel's picture

I think you guys are missing the point.

Comments are there to make the code maintainable. As most costs
associated with development are maintenance (>80%), it's a very good
idea to have comments to tell the hung-over developer debugging the
code in the middle of the night what in the world is going on. Many
times that person will be you.

As a counter example to Ufsin, take a look at this single line from
the code I just cited:
S SD=$J(SD,2,4)

I am at a total loss trying to explain why is the programmer
formatting the Start Date ($JUSTIFY is typically used in formatting
output). Running it in the command line eventually reveals that the
programmer is stripping seconds away and leaving minutes and hours.
But how would I know that? It's a trick, and in complex code when I am
troubleshooting a problem I can't spend all my time executing every
single line of code to figure out what it does.

Ufsin, I don't think many people will agree with your position. I have
seen some people hold your position, but my impression is that they
are the minority. When code is changed, comments should be updated.

I have been criticized before for writing a novel with my code. Yes, I
over do it, but in a language like Mumps whose readability is not
great, I think it's essential.

For a good example of how to comment code well, take a look the source
for coffeescript:

http://coffeescript.org/documentation/docs/grammar.html

Sam

On Mon, May 6, 2013 at 9:01 AM, Christopher.Edwards
<Christopher.Edwards@krminc.com> wrote:
> I believe this is the critical section of the SAC that Sam is referring to:
>
> 2.10.4.2 Any supported references or routines invoked initially from an
> option or protocol should contain comments explaining the functionality and
> all input and output variables.
>
> There is a debate as to wether documentation should live with the code or be
> in separate documentation. I tende to lean on the side of putting the
> documentation as close to the intended audience as possible.
>
> In this case I'd vote (if it matters :) ) for a comment describing the Input
> and Output variables for the API as these will be used by developers. If
> separate documentation is needed it could be generated by doxoygen,
> docutils, etc.
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions: http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post:
> http://www.osehra.org/og_mailinglist/unsubscribe/1846

like0

A small rant on the lack of comments...

Wes Turner's picture

Sam,

I'm not disagreeing with you, so I am not sure what point I am missing.
Comments are good. If you add a document generator, then you have a
convenient mechanism for reviewing the major ones (parameters, entry
points, etc.) and you ensure that the code, documentation and comments all
stay in sync ...

On Mon, May 6, 2013 at 12:22 PM, Sam Habiel <sam.habiel@gmail.com> wrote:

> I think you guys are missing the point.
>
> Comments are there to make the code maintainable. As most costs
> associated with development are maintenance (>80%), it's a very good
> idea to have comments to tell the hung-over developer debugging the
> code in the middle of the night what in the world is going on. Many
> times that person will be you.
>
> As a counter example to Ufsin, take a look at this single line from
> the code I just cited:
> S SD=$J(SD,2,4)
>
> I am at a total loss trying to explain why is the programmer
> formatting the Start Date ($JUSTIFY is typically used in formatting
> output). Running it in the command line eventually reveals that the
> programmer is stripping seconds away and leaving minutes and hours.
> But how would I know that? It's a trick, and in complex code when I am
> troubleshooting a problem I can't spend all my time executing every
> single line of code to figure out what it does.
>
> Ufsin, I don't think many people will agree with your position. I have
> seen some people hold your position, but my impression is that they
> are the minority. When code is changed, comments should be updated.
>
> I have been criticized before for writing a novel with my code. Yes, I
> over do it, but in a language like Mumps whose readability is not
> great, I think it's essential.
>
> For a good example of how to comment code well, take a look the source
> for coffeescript:
>
> http://coffeescript.org/documentation/docs/grammar.html
>
> Sam
>
> On Mon, May 6, 2013 at 9:01 AM, Christopher.Edwards
> <Christopher.Edwards@krminc.com> wrote:
> > I believe this is the critical section of the SAC that Sam is referring
> to:
> >
> > 2.10.4.2 Any supported references or routines invoked initially from an
> > option or protocol should contain comments explaining the functionality
> and
> > all input and output variables.
> >
> > There is a debate as to wether documentation should live with the code
> or be
> > in separate documentation. I tende to lean on the side of putting the
> > documentation as close to the intended audience as possible.
> >
> > In this case I'd vote (if it matters :) ) for a comment describing the
> Input
> > and Output variables for the API as these will be used by developers. If
> > separate documentation is needed it could be generated by doxoygen,
> > docutils, etc.
> >
> > --
> > Full post: http://www.osehra.org/content/small-rant-lack-comments
> > Manage my subscriptions:
> http://www.osehra.org/og_mailinglist/subscriptions
> > Stop emails for this post:
> > http://www.osehra.org/og_mailinglist/unsubscribe/1846
>
>
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions:
> http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post:
> http://www.osehra.org/og_mailinglist/unsubscribe/1846
>
>

--
Wesley D. Turner, Ph.D.
Kitware, Inc.
Technical Leader
28 Corporate Drive
Clifton Park, NY 12065-8662
Phone: 518-881-4920

like0

A small rant on the lack of comments...

Roy Gaber's picture

I think your point is well understood Sam, it is a matter of semantics
now.

Which leads us to coding style, while the standards and conventions may
guide a developer along the way by dictating certain rules, it does not
address readability. Many of us M(UMPS) developers, as with any other
language, develop our own coding style, while all variations may
accomplish the same task, some may decide to write

SET SD=$JUSTIFY(SD,2,4) - readable

others may decide to use S SD=$J(SD,2,4) - not readable, but takes up
less space, given that the SACC dictated routine size in the early days
due to RAM (partition size) constraints and other limitations, and the
need to standardize nationally developed code, the practice was brought
forward in spite of technological advances.

It gets easy after a while Sam. :)

-----Original Message-----
From: Apache [mailto:apache@groups.osehra.org] On Behalf Of Sam Habiel
Sent: Monday, May 06, 2013 12:22 PM
To: ehr-refactoring-services@groups.osehra.org
Subject: Re: [ehr-refactoring-services] A small rant on the lack of
comments...

I think you guys are missing the point.

Comments are there to make the code maintainable. As most costs
associated with development are maintenance (>80%), it's a very good
idea to have comments to tell the hung-over developer debugging the code
in the middle of the night what in the world is going on. Many times
that person will be you.

As a counter example to Ufsin, take a look at this single line from the
code I just cited:
S SD=$J(SD,2,4)

I am at a total loss trying to explain why is the programmer formatting
the Start Date ($JUSTIFY is typically used in formatting output).
Running it in the command line eventually reveals that the programmer is
stripping seconds away and leaving minutes and hours.
But how would I know that? It's a trick, and in complex code when I am
troubleshooting a problem I can't spend all my time executing every
single line of code to figure out what it does.

Ufsin, I don't think many people will agree with your position. I have
seen some people hold your position, but my impression is that they are
the minority. When code is changed, comments should be updated.

I have been criticized before for writing a novel with my code. Yes, I
over do it, but in a language like Mumps whose readability is not great,
I think it's essential.

For a good example of how to comment code well, take a look the source
for coffeescript:

http://coffeescript.org/documentation/docs/grammar.html

Sam

On Mon, May 6, 2013 at 9:01 AM, Christopher.Edwards
<Christopher.Edwards@krminc.com> wrote:
> I believe this is the critical section of the SAC that Sam is
referring to:
>
> 2.10.4.2 Any supported references or routines invoked initially from
> an option or protocol should contain comments explaining the
> functionality and all input and output variables.
>
> There is a debate as to wether documentation should live with the code

> or be in separate documentation. I tende to lean on the side of
> putting the documentation as close to the intended audience as
possible.
>
> In this case I'd vote (if it matters :) ) for a comment describing the

> Input and Output variables for the API as these will be used by
> developers. If separate documentation is needed it could be generated
> by doxoygen, docutils, etc.
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions:
> http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post:
> http://www.osehra.org/og_mailinglist/unsubscribe/1846

like0

A small rant on a small rant on the lack of comments

Bryan Lucas's picture

 A comment on  comments from a personal and historical viewpoint:  Some additional reasons for the lack of commenting and readable code are that a lot of what is within VistA was written by programmers at sites only for local use and then spread to other sites by word of mouth because a particular program was found useful.  Very often the author may well have had no formal programming education and, indeed, taught themselves to code in M.  IF they were aware of the SACC then there is, as mentioned, a restriction on entry-point and variable name sizes, which is compounded by having to “namespace” those names, meaning that you had five characters from which to form a variable name.  I personally at times spent enough time trying to form meaningful variable names from the five available characters that I would eventually give-up and start simply incrementing (AVFVAR01, AVFVAR02, etc).   Those local programmers were also unlikely to be “just” programmers and coding projects had to be done when you weren’t installing patches, researching errors, teaching users, maybe even fixing printers, replacing workstations, and various other duties.  A programming project with a deadline was also most often plagued by drifting requirements, mid-project requests for additional functionality, output format changes, etc.  I always intended to go back and create better comments and documentation, unfortunately project time was spent restructuring programs to meet new requirements; and after the project there would always be new projects with deadlines to prevent bettering documentation post-deployment.  And since the program was working and those who needed to use it knew how to do so, the priority of re-commenting and documenting fell very low.  I have been known to say, “Would you rather have a program that does everything you want without errors, or good documentation?”  Unfortunately, that was often a valid question. 

As for readability due to the use of command abbreviations, that may well be because in Cache Studio you can expand and compress commands via the IDE.  Typically though it was to delay the onset of Carpal Tunnel Syndrome; because when needed, simply telling Studio to “expand commands” did so, and then they were again compressed before saving to save space.  Familiarity with the commands also compounds this.  I will admit that there are commands that when expanded I have to give a second look to realize what it is because I am so used to seeing it in its abbreviated form.

Certainly, APIs should at minimum explain input parameters and output:

AVDMYAPI(AVDVAR01,AVDVAR02) ;Add, delete, or modify a record
  ;Expects:
  ; AVDVAR01: called by value, ien from File "BUNCHFRECORDS” (757003), or
  ; node in ^TMP(“AVDPROG”,node) where new record data exists
  ; AVDVAR02: called: by value, operation to perform; Set of (1:Add, 2:Delete, 3:Modify)
  ;Returns:
  ; AVFRESLT: 0:Operation failed, >0: ien of record added, deleted, or modified

I DO think that API comments have improved greatly through the years in VA code.

In any case, I think the idea of a documentation generator is great; perhaps even a documentation repository to which a particular routine or API’s  “extended commentary” can be accessed via a hyperlink from the in-code comments.  A documentation generator may even drive improvements in code comments as programmers strive to make their generated documentation look better and more robust.  And, I promise that when I get the free-time, I will go back and add better in-code comments and external documentation for anything I have written.  ;)

like0

A small rant on the lack of comments...

Roy Gaber's picture

Functionality, ease of use, and efficient is the goal of all developers, at least it should be.

I think the issue is that VistA is becoming more mainstream, more accepted by the healthcare IT community, and there are not that many M(UMPS) programmers outside the VA who have as much exposure to the VistA codebase as those within the organization, this seems to be at least one, if not the major source of frustration. In the beginning it was all foreign to me, coming from a C background, many years later, all of them as a VA programmer, it is second nature and reading any bit of code is like reading the daily newspaper.

Don’t hear me wrong, documenting is a good thing, where necessary.

From: Apache [mailto:apache@groups.osehra.org] On Behalf Of BLVA2011
Sent: Monday, May 06, 2013 10:57 PM
To: EHR Refactoring Services
Subject: Re: [ehr-refactoring-services] A small rant on the lack of comments...

A comment on comments from a personal and historical viewpoint: Some additional reasons for the lack of commenting and readable code are that a lot of what is within VistA was written by programmers at sites only for local use and then spread to other sites by word of mouth because a particular program was found useful. Very often the author may well have had no formal programming education and, indeed, taught themselves to code in M. IF they were aware of the SACC then there is, as mentioned, a restriction on entry-point and variable name sizes, which is compounded by having to “namespace” those names, meaning that you had five characters from which to form a variable name. I personally at times spent enough time trying to form meaningful variable names from the five available characters that I would eventually give-up and start simply incrementing (AVFVAR01, AVFVAR02, etc). Those local programmers were also unlikely to be “just” programmers and coding projects had to be done when you weren’t installing patches, researching errors, teaching users, maybe even fixing printers, replacing workstations, and various other duties. A programming project with a deadline was also most often plagued by drifting requirements, mid-project requests for additional functionality, output format changes, etc. I always intended to go back and create better comments and documentation, unfortunately project time was spent restructuring programs to meet new requirements; and after the project there would always be new projects with deadlines to prevent bettering documentation post-deployment. And since the program was working and those who needed to use it knew how to do so, the priority of re-commenting and documenting fell very low. I have been known to say, “Would you rather have a program that does everything you want without errors, or good documentation?” Unfortunately, that was often a valid question.

As for readability due to the use of command abbreviations, that may well be because in Cache Studio you can expand and compress commands via the IDE. Typically though it was to delay the onset of Carpal Tunnel Syndrome; because when needed, simply telling Studio to “expand commands” did so, and then they were again compressed before saving to save space. Familiarity with the commands also compounds this. I will admit that there are commands that when expanded I have to give a second look to realize what it is because I am so used to seeing it in its abbreviated form.

Certainly, APIs should at minimum explain input parameters and output:

AVDMYAPI(AVDVAR01,AVDVAR02) ;Add, delete, or modify a record
;Expects:
; AVDVAR01: called by value, ien from File "BUNCHFRECORDS” (757003), or
; node in ^TMP(“AVDPROG”,node) where new record data exists
; AVDVAR02: called: by value, operation to perform; Set of (1:Add, 2:Delete, 3:Modify)
;Returns:
; AVFRESLT: 0:Operation failed, >0: ien of record added, deleted, or modified

I DO think that API comments have improved greatly through the years in VA code.

In any case, I think the idea of a documentation generator is great; perhaps even a documentation repository to which a particular routine or API’s “extended commentary” can be accessed via a hyperlink from the in-code comments. A documentation generator may even drive improvements in code comments as programmers strive to make their generated documentation look better and more robust. And, I promise that when I get the free-time, I will go back and add better in-code comments and external documentation for anything I have written. ;)

--
Full post: http://www.osehra.org/content/small-rant-lack-comments
Manage my subscriptions: http://www.osehra.org/og_mailinglist/subscriptions
Stop emails for this post: http://www.osehra.org/og_mailinglist/unsubscribe/1846

like0

I like the newspaper analogy.

Ferdinand Frankson's picture

I like the newspaper analogy. Just one question: when you are having your daily read do you prefer the newspaper format to be like this:

GETEQPD1(A) N E,D S E=$O(^ENG(6914,"EE",A,"")),C=C+1,^TMP(R_RE,$J,C,0)=A_U_$$GET1^DIQ(6914,E,24)_U_$$GET1^DIQ(6914,E,181999.9)_U_$$GET1^DIQ(6914,E,4)_U_$$GET1^DIQ(6914,E,1)_U_$$GET1^DIQ(6914,E,6,"I")_U_$$GET1^DIQ(6914,E,20)_U_$$GET1^DIQ(6914,E,5)_U_$$GET1^DIQ(6914,E,12)_U_$$GET1^DIQ(6914,E,.6,"I")_U_$$GET1^DIQ(6914,E,23,"I")_U_$$GET1^DIQ(6914,E,3)_U_$$GET1^DIQ(6914,E,19)_U_$$GET1^DIQ(6914,E,21)_U_$$GET1^DIQ(6914,E,18) Q

or like this:

GETEQPD1(AEMSID) ; get data for one item ; N EIEN,LOCID,RTAGID,MODEL,MANUF,CATEG,SERNO,USESTAT,VALUE,DATA N ENTDAT,MOVDAT,EQNAM,CMR,SERVICE,CATSTKNO S EIEN=$O(^ENG(6914,"EE",AEMSID,"")) S LOCID=$$GET1^DIQ(6914,EIEN,24) ; location S RTAGID=$$GET1^DIQ(6914,EIEN,181999.9) ; RTLS tag identifier S MODEL=$$GET1^DIQ(6914,EIEN,4) ; model S MANUF=$$GET1^DIQ(6914,EIEN,1) ; manufacturer S CATEG=$$GET1^DIQ(6914,EIEN,6,"I") ; equipment category S SERNO=$$GET1^DIQ(6914,EIEN,5) ; serial number S USESTAT=$$GET1^DIQ(6914,EIEN,20) ; use status S VALUE=$$GET1^DIQ(6914,EIEN,12) ; asset value S ENTDAT=$$GET1^DIQ(6914,EIEN,.6,"I") ; date asset entered in AEMS S MOVDAT=$$GET1^DIQ(6914,EIEN,23,"I") ; date of last movement S EQNAM=$$GET1^DIQ(6914,EIEN,3) ; equipment name S CMR=$$GET1^DIQ(6914,EIEN,19) ; cmr S SERVICE=$$GET1^DIQ(6914,EIEN,21) ; service pointer S CATSTKNO=$$GET1^DIQ(6914,EIEN,18) ; category stock number S DATA=AEMSID_U_LOCID_U_RTAGID_U_MODEL_U_MANUF_U_CATEG_U_USESTAT S DATA=DATA_U_SERNO_U_VALUE_U_ENTDAT_U_MOVDAT_U_EQNAM_U_CMR S DATA=DATA_U_SERVICE_U_CATSTKNO S RECCT=RECCT+1 S ^TMP(RNS_REQDATA,$J,RECCT,0)=DATA Q

 ?

like0

A small rant on the lack of comments...

Roy Gaber's picture

Does not really matter to me, they read the same, like I said like any other language it is a coding style, and like learning a new language it takes some time and effort to master it. Don’t get me wrong, it is much easier for beginners to read the second bit easier than the first, but a seasoned Mumpster would read both the same, at least I do and I don’t think I am unique.

From: Apache [mailto:apache@groups.osehra.org] On Behalf Of ferdi
Sent: Tuesday, May 07, 2013 9:59 AM
To: EHR Refactoring Services
Subject: Re: [ehr-refactoring-services] A small rant on the lack of comments...

I like the newspaper analogy. Just one question: when you are having your daily read do you prefer the newspaper format to be like this:

GETEQPD1(A) N E,D S E=$O(^ENG(6914,"EE",A,"")),C=C+1,^TMP(R_RE,$J,C,0)=A_U_$$GET1^DIQ(6914,E,24)_U_$$GET1^DIQ(6914,E,181999.9)_U_$$GET1^DIQ(6914,E,4)_U_$$GET1^DIQ(6914,E,1)_U_$$GET1^DIQ(6914,E,6,"I")_U_$$GET1^DIQ(6914,E,20)_U_$$GET1^DIQ(6914,E,5)_U_$$GET1^DIQ(6914,E,12)_U_$$GET1^DIQ(6914,E,.6,"I")_U_$$GET1^DIQ(6914,E,23,"I")_U_$$GET1^DIQ(6914,E,3)_U_$$GET1^DIQ(6914,E,19)_U_$$GET1^DIQ(6914,E,21)_U_$$GET1^DIQ(6914,E,18) Q

or like this:

GETEQPD1(AEMSID) ; get data for one item
;
N EIEN,LOCID,RTAGID,MODEL,MANUF,CATEG,SERNO,USESTAT,VALUE,DATA
N ENTDAT,MOVDAT,EQNAM,CMR,SERVICE,CATSTKNO
S EIEN=$O(^ENG(6914,"EE",AEMSID,""))
S LOCID=$$GET1^DIQ(6914,EIEN,24) ; location
S RTAGID=$$GET1^DIQ(6914,EIEN,181999.9) ; RTLS tag identifier
S MODEL=$$GET1^DIQ(6914,EIEN,4) ; model
S MANUF=$$GET1^DIQ(6914,EIEN,1) ; manufacturer
S CATEG=$$GET1^DIQ(6914,EIEN,6,"I") ; equipment category
S SERNO=$$GET1^DIQ(6914,EIEN,5) ; serial number
S USESTAT=$$GET1^DIQ(6914,EIEN,20) ; use status
S VALUE=$$GET1^DIQ(6914,EIEN,12) ; asset value
S ENTDAT=$$GET1^DIQ(6914,EIEN,.6,"I") ; date asset entered in AEMS
S MOVDAT=$$GET1^DIQ(6914,EIEN,23,"I") ; date of last movement
S EQNAM=$$GET1^DIQ(6914,EIEN,3) ; equipment name
S CMR=$$GET1^DIQ(6914,EIEN,19) ; cmr
S SERVICE=$$GET1^DIQ(6914,EIEN,21) ; service pointer
S CATSTKNO=$$GET1^DIQ(6914,EIEN,18) ; category stock number
S DATA=AEMSID_U_LOCID_U_RTAGID_U_MODEL_U_MANUF_U_CATEG_U_USESTAT
S DATA=DATA_U_SERNO_U_VALUE_U_ENTDAT_U_MOVDAT_U_EQNAM_U_CMR
S DATA=DATA_U_SERVICE_U_CATSTKNO
S RECCT=RECCT+1
S ^TMP(RNS_REQDATA,$J,RECCT,0)=DATA
Q

?

--
Full post: http://www.osehra.org/content/small-rant-lack-comments
Manage my subscriptions: http://www.osehra.org/og_mailinglist/subscriptions
Stop emails for this post: http://www.osehra.org/og_mailinglist/unsubscribe/1846

like0

Is in code documentation always good?

Afsin Ustundag's picture

First of all I agree with documenting each label/API entry inputs, output, and what it does before or right after.  I do not think there is any contention there.  I think we are missing that in some of the refactored code where we concentrated on the API document instead and we should fix that.

In code documentation however is not that black and white.  I might be in the minority but I've known a lot of good programmers share the same view and if you google on in code documentation you will see a lot people expressing the same opinions.  Your "code" should be your best documentation.  If this was a Python forum I would tell you that if you need a novel to describe what your code is doing then you should review your code, use better class hierarchy, better variable names, better method names, etc. rather than spending time on writing your novel.

Now I realize that Mumps is no Python and programmers are handicapped by restrictions on method names, variable names, etc. so there needs to be more in code documentation.  However I disagree with the notion that the more in code documentation the better.  In code documentation should be in there only if it is necessary and certainly should not be superfluous.  So you just cannot compare two routines and say one with in code documentation is better.

These are some comments from web that I agree with.  Lots of others you can find by simple search.  I will add a bug for our  team to add brief description for inputs, outputs and what it does for most labels and as you and other people point out pieces of our code (like the one above) that needs to be documented we will do.

"Good comments don't repeat the code or explain it. They clarify its intent. Comments should explain, at a higher level of abstraction than the code, what you're trying to do."

"The in code documentation is often superfluous. Don’t comment stuff that is evident from the code. If the intention is not evident, fix the naming of the things in the code until the intention is evident. Keep the in code documentation to a minimum, I usually only do the XML comments for intellisense. They make sense as they show up when a function is called, without looking at actual code of the function."

like0

Creating well documented,

Ferdinand Frankson's picture

Creating well documented, readable, maintainable, and adaptable code is good software engineering practice. There is nothing to prevent OSEHRA devolopers from exceeding SACC standards in those cases where that is necessary order to achieve this.

like0

A small rant on the lack of comments...

Rafael Richards MD, MS's picture

I agree with all three of you (Wes, Sam, Ufsin) on different points.

Wes- on automation that matches the current state of the art; keeping documentation as close as possible to the code.

Sam- on thorough documentation

Afsin - on writing clear readable code in the first place.

I do disagree with Afsin on one point- that it is okay to leave code in its single letter form. That is not readable, and certainly not for the person who did not write it. It contradicts the assertion that the code should be so clear it is self- documenting.

Either way, I do think we all agree that more screen is going to have yo be be spent with readable code, or readable comments.

And these days, what professional programmer is *not* using a dual monitor setup with 10x the screen real estate, or 100x the memory and processors of the folks who wrote this code originally?

I think the best way to resolve (1) granularity and style of documentation and (2) coherence and readability of "self documenting" code is to see some examples. Can you post some specific examples of what we are talking about?

An outcome of this hopefully will be some best practices recommendations - based on today's display and documentation technology - that we can forward to the VA. This is precisely why they created OSEHRA.

Rafael

On May 6, 2013, at 2:40 PM, austundag <austundag@raygroupintl.com> wrote:

> First of all I agree with documenting each label/API entry inputs, output, and what it does before or right after. I do not think there is any contention there. I think we are missing that in some of the refactored code where we concentrated on the API document instead and we should fix that.
>
> In code documentation however is not that black and white. I might be in the minority but I've known a lot of good programmers share the same view and if you google on in code documentation you will see a lot people expressing the same opinions. Your "code" should be your best documentation. If this was a Python forum I would tell you that if you need a novel to describe what your code is doing then you should review your code, use better class hierarchy, better variable names, better method names, etc. rather than spending time on writing your novel.
>
> Now I realize that Mumps is no Python and programmers are handicapped by restrictions on method names, variable names, etc. so there needs to be more in code documentation. However I disagree with the notion that the more in code documentation the better. In code documentation should be in there only if it is necessary and certainly should not be superfluous. So you just cannot compare two routines and say one with in code documentation is better.
>
> These are some comments from web that I agree with. Lots of others you can find by simple search. I will add a bug for our team to add brief description for inputs, outputs and what it does for most labels and as you and other people point out pieces of our code (like the one above) that needs to be documented we will do.
>
> "Good comments don't repeat the code or explain it. They clarify its intent. Comments should explain, at a higher level of abstraction than the code, what you're trying to do."
>
> "The in code documentation is often superfluous. Don’t comment stuff that is evident from the code. If the intention is not evident, fix the naming of the things in the code until the intention is evident. Keep the in code documentation to a minimum, I usually only do the XML comments for intellisense. They make sense as they show up when a function is called, without looking at actual code of the function."
>
> --
> Full post: http://www.osehra.org/content/small-rant-lack-comments
> Manage my subscriptions: http://www.osehra.org/og_mailinglist/subscriptions
> Stop emails for this post: http://www.osehra.org/og_mailinglist/unsubscribe/1846

like0

Examples

Afsin Ustundag's picture

I will give an example on the line Sam pointed out

S SD=$J(SD,2,4)

A VA programmer added this in the last version of VISTA-FOIA and it is what I would call an "overly creative" line so I agree with Sam that it should be documented.  However complex a statement it is I would still categorize the following comment as superfluous

; Justify SD to remove the last two digits

S SD=$J(SD,2,4)

 

This is a much better comment

; Remove the seconds from Fileman date/time

S SD=$J(SD,2,4)

 

But you can still do much better reorganizing your code and moving the logic to an extrinsic function

; Remove seconds

REMSECS(SD)

 Q $J(SD,2,4)

....

S SD=$$REMSECS(SD)

 

Here the name restrictions in Mumps come into play.  If you were allowed to use more than 8 characters you would write this as

S SD=$$REMOVESECONDS(SD)

and you do not even need to look at the extrinsic function while reading the code.

So instead of using an in code comment your code makes your intent clear.  This way next time when VA decides to store the seconds as well the next programmer does not have to remember/bother to change the in code comment as well.  And programmers forget changing in code comments more often than you think especially when they are in time crunch. 

like0

A small rant on the lack of comments...

Roy Gaber's picture

IMHO, this would be considered unnecessary commenting, regarding the function then yes by all means, it should be documented.

From: Apache [mailto:apache@groups.osehra.org] On Behalf Of austundag
Sent: Monday, May 06, 2013 5:47 PM
To: EHR Refactoring Services
Subject: Re: [ehr-refactoring-services] A small rant on the lack of comments...

I will give an example on the line Sam pointed out

S SD=$J(SD,2,4)

A VA programmer added this in the last version of VISTA-FOIA and it is what I would call an "overly creative" line so I agree with Sam that it should be documented. However complex a statement it is I would still categorize the following comment as superfluous

; Justify SD to remove the last two digits

S SD=$J(SD,2,4)

This is a much better comment

; Remove the seconds from Fileman date/time

S SD=$J(SD,2,4)

But you can still do much better reorganizing your code and moving the logic to an extrinsic function

; Remove seconds

REMSECS(SD)

Q $J(SD,2,4)

....

S SD=$$REMSECS(SD)

Here the name restrictions in Mumps come into play. If you were allowed to use more than 8 characters you would write this as

S SD=$$REMOVESECONDS(SD)

and you do not even need to look at the extrinsic function while reading the code.

So instead of using an in code comment your code makes your intent clear. This way next time when VA decides to store the seconds as well the next programmer does not have to remember/bother to change the in code comment as well. And programmers forget changing in code comments more often than you think especially when they are in time crunch.

--
Full post: http://www.osehra.org/content/small-rant-lack-comments
Manage my subscriptions: http://www.osehra.org/og_mailinglist/subscriptions
Stop emails for this post: http://www.osehra.org/og_mailinglist/unsubscribe/1846

like0