Written by

Senior Startups and Community Programs Manager at InterSystems Corporation
Discussion Evgeny Shvarov · Jan 24, 2018

For and If with one line: to brace or not to brace

Hi Community!

Coding guidelines discussion. Consider you have For with one command in the cycle. Here are the options:

One

For i=1:1:1000 set ^Test(i)=""

Two

For i=1:1:1000 {
 set ^Test(i)=""
}

Same for the "If":

if a set b=a

or

if a {
 set b=a
}

Your choice? Or maybe even other options? 

Comments

Eduard Lebedyuk · Jan 24, 2018

For one-line if I prefer postconditionals:

set:a b=a

 

As for For - the option with braces. It's more readable. And one line inside For loop quickly and often becomes several lines, so braces become mandatory anyway.

0
Evgeny Shvarov  Jan 24, 2018 to Eduard Lebedyuk

But...

Can one condition in "postconditionals" "quickly and often" convert into several lines?

Maybe postconditionals shoud be If with braces too?

0
Eduard Lebedyuk  Jan 24, 2018 to Evgeny Shvarov

Postconditionals are to be used only for the most simple checks, for example:

set:a="" a=10

instead of

if a="" {
  set a=10
}

But several conditions (or several lines inside) should be as if, I agree.

0
Fabian Haupt  Apr 27, 2018 to Eduard Lebedyuk

for several conditions there's always $S too.

0
Jorge de la Garza  Jan 24, 2018 to Eduard Lebedyuk

I tend to use "if" rather than postconditionals because it makes the code read more like spoken language, and I think it's easier to understand for those who are not seasoned Caché veterans.

However I make exceptions for "quit" and "continue":

quit:$$$ISERR(tSC)

continue:x'=y

I like to have these out in front, rather than buried in a line somewhere, so that when I'm scanning code I can easily see "this is where the execution will branch if a certain condition is met."

0
John Murray  Jan 26, 2018 to Jorge de la Garza

I also like this style, as it helps me spot the lines where execution at the current level might terminate.

0
Neerav Verma  Feb 4, 2020 to Eduard Lebedyuk

HI Eduard, I always felt PostConditionals are just plain confusing.  It can easily be argued that it's one line and simple but in my personal opinion I would rather use an if / else with braces with proper indentation. We are not living in mumps time, so do have the luxury of having some extra spaces and tabs.

0
Eduard Lebedyuk  Feb 4, 2020 to Neerav Verma

Hi Neerav!

I agree that postconditionals are an acquired taste :)

0
Anthony Ennis  Jan 16, 2024 to Neerav Verma

Unless you have to maintain code written in the 70s. So it is better to be versed in their use and write new code that fits the style of the old code we maintain.

0
Kurro Lopez · Jan 24, 2018

Yep, brace always.

Because you will never know if you need to update your condition or chain another. The brace shows what is the scope of the condition, also for loops, for, for each, etc.

It is a good practices and it is more readable.

As Eduard Lebedyuk   has commented, a single IF for use to set a variable, the postconditionals is a good idea, coz you know the condition before to set.

0
Otto Medin  Jan 24, 2018 to Kurro Lopez

Agreed, and if you want to have your cake and eat it, too:

if (a) { set b = a }
0
Peter Steiwer  Jan 24, 2018 to Kurro Lopez

Brace always for me too, unless I am writing some quick debugging code or something along those lines

0
Warlin Garcia  Jan 24, 2018 to Kurro Lopez

Agree with brace always. This really helps with new developers as the syntax is something they've seen before in other languages. 

0
Robert Cemper · Jan 24, 2018

Postcondition is preferred
especially  quit:error     or   continue:ignore

for i=1:1:5 write i," "     is as fine as    for i=1:1:5 {  write i," " } write "end"

but this is really bad:

         f i=1:1:5 d  w "? "
          .w i," "
         w "end"
0
Kurro Lopez  Jan 24, 2018 to Robert Cemper

OMG!!! What is that!!!

It seems a KGB spy code !!! laughlaughlaugh

0
Robert Cemper  Jan 24, 2018 to Kurro Lopez

I think they named it M(umps)

laugh

0
Neerav Verma  Feb 4, 2020 to Robert Cemper

hahaha exactly.  this single alphabet codes goes top of my head. need extra cups of coffee when debugging a code like that. Meant only for the masters. 

0
Danny Wijnschenk · Jan 25, 2018

Besides all that is mentioned, the If without braces has a side-effect that the If with braces does not have : it affects the system variable $Test.

USER>set $test=0
 
USER>w $test
0
USER>if 1=1 { write "one" }
one
USER>w $test
0
USER>if 1=1 write "one"
one
USER>w $test
1
USER>

Not important, only if you use the ELSE (legacy) command...

0
Kurro Lopez  Jan 25, 2018 to Danny Wijnschenk

Indeed... the brace delimits the scope of the operation

0
Evgeny Shvarov  Jan 25, 2018 to Danny Wijnschenk

Thanks, Danny!

Is there any "non-legacy" case when $test can be useful?

I mean, should we consider using $test in a code as a bad practice?

0
Warlin Garcia  Jan 25, 2018 to Evgeny Shvarov

If you want to check on the result of a LOCK command for example 

LOCK +<SOMERESOURCE>:<timeout>

IF $T {do something}

So I wouldn't call it bad practice just yet.

0
Danny Wijnschenk  Jan 25, 2018 to Warlin Garcia

Yes, same here, i also use following code a lot:

Open <some device>:"R":0 Else  Write "could not open device" Quit

I always use Else on the same line just after the command that changes $test,

having too much instructions between them creates problems. 

0
Evgeny Shvarov  Jan 25, 2018 to Warlin Garcia

Oh right, forgot about that. 

0
Joel Solon  Jan 31, 2018 to Warlin Garcia

I would do it this way to avoid Else:

Open <some device>:"R":0 If '$test { Write "could not open device" Quit }

0
Joel Solon  Jan 31, 2018 to Danny Wijnschenk

C'mon, let's stop mentioning old features like argumentless Do and legacy ELSE

There's just no point in doing so.

0
Danny Wijnschenk  Jan 31, 2018 to Joel Solon

In the real world, there are lots of programs still using this style, every developer using Caché Object Script should be able to read this and understand the side-effects, pitfalls, even if we recommend to not use it anymore... 

0
Robert Cemper  Jan 31, 2018 to Danny Wijnschenk

totally right!!
there is more old code out than you could / would like to imagine
And Caché is backward compatible for 40 years (since DSM V1)

New developers should not write it. But they need to understand it. Or at least recognize it.
It's pretty similar to reading the bible in original language with no translation. 

0
Joel Solon  Jan 31, 2018 to Robert Cemper

Danny and Robert, of course I understand what you are saying (we are the same generation!). Also, I am not trying to be the Developer Community police.

In posts that are asking "what's the recommended way to do this?" we should answer that question. There's no point in mentioning old syntax in that context. Saying "don't do this, but you might see it someday" helps the subset of people that might work with old code, but doesn't help and may confuse the people that are working with new code. It doesn't belong.

On the other hand, if there's a post asking "what are some older/confusing syntaxes I might encounter when working with M or ObjectScript?" we should answer that question. We all have examples of that.

0
Robert Cemper  Jan 31, 2018 to Joel Solon

I wonder if it would be possible to have a more rigid syntax check (now in Atelier / and switchable)
to enforce all kind of brackets (eg: condition like in JS). 

I tried to recommend this while teaching COS myself.
With very limited success as some Dinos ignored it.
 

0
Thomas Carroll  Jan 31, 2018 to Danny Wijnschenk

Everyday I read legacy code like this, but if we're talking about the ideal (like coding guidelines) we want to avoid these. I agree that every Object Script developer should be able to read this code, but they should also know that there are better modern ways to write this. 

I think that part of growing the community of Object Script developers is to lower barriers to entry. Having worked with a lot of new Object Script developers (and been one myself), one of the largest barriers to entry is the prevalence of cryptic legacy syntax in existing code. 

Remember, just because we can read it doesn't mean that everyone can.

0
Jon Willeke  Feb 1, 2018 to Joel Solon

Argumentless DO is still the simplest way to introduce a scope to NEW a variable, including $namespace or $roles. Yes, you can extract that out to another procedure or method, but I often prefer to do it inline.

0
John Murray  Feb 2, 2018 to Jon Willeke

Good point Jon. Here's a concrete example, taken from the CheckLinkAccess classmethod of %CSP.Portal.Utils (2017.2 version). I have done the macro expansion myself and highlighted the expansions:

  Set tHaveAllRole = 0
  Do 
  . New $roles
  . Set $roles=""
  . If ($extract($roles,1,$length("%All"))="%All")Set tHaveAllRole = 1
  If tHaveAllRole {

  ...

0
Robert Cemper  Feb 2, 2018 to John Murray

yesyesyesyesyesyes

Congratulations!

I new I had seen something similar

0
Steven Hobbs  Jan 25, 2024 to John Murray

I always wanted to extend argumentless  DO so that you could get a new scope without needing the fairly ugly dot-indentation lines.  Then you write something like

Do {
   New $roles
   Set $roles=""
   If ($extract($roles,1,$length("%All"))="%All")Set tHaveAllRole = 1
}

You could even write the whole thing on one line like

Do {New $roles  Set $roles=""  If ($extract($roles,1,$length("%All"))="%All")  Set tHaveAllRole = 1}

0
Julius Kavay  Jan 25, 2024 to Steven Hobbs

I hope someone is already working on that extension and it's scheduled for the upcoming release

0
Steven Hobbs  Jan 25, 2024 to Steven Hobbs

Well, my DO command suggestion won't work in ObjectScript because ObjectScript has extended the MUMPS DO command to accept syntax for the
   DO { code } WHILE expr,expr,...
statement.  The code block in the DO-WHILE extension does not assume the code block opens a new variable scope.

However, like most languages designed in the 1950s and 1960s, (including FORTRAN, BASIC and PL/I, but not C) MUMPS does not have reserved words.  Therefore, any newly added command can start with a new command word.  So we could have BLOCK { code } or SCOPE { code } as new commands that open a new variable scope.  We might be able to find a better command word than BLOCK or SCOPE.  We could even consider NEW { code }, although it might be considered confusing having an additional NEW command (one with a curly brace code block along with one with no arguments, one with list of argument names and one with a list of names surrounded by a pair of round braces).

0
Julius Kavay  Jan 26, 2024 to Steven Hobbs

You have right, do {code} and do {code} while expr are mutually exclusive.

I thought more something like this do {{code}} (no spaces between curly braces) or, as you suggested, a new keyword like WRAP {...}, of course, BLOCK {...} and SCOPE {...} are also acceptable.

By the way, we already have an alternative for argumentless DO
do {code} while 0
It looks ugly and confusing and
- does not preseve $T
- does not establich a new stack level
but one can get over both easily respective do a work-around.

0
Evgeny Shvarov  Jan 26, 2024 to Steven Hobbs

Looks great!

Do you want to submit an idea on Ideas Portal? So people will vote for the enhancement?

0
Evgeny Shvarov  Jan 26, 2024 to Steven Hobbs

Today to achieve the similar people do:

X "New $roles Set $roles="""" If ($extract($roles,1,$length(""%All""))=""%All"") Set tHaveAllRole =1"
0
Robert Barbiaux  Jan 30, 2024 to Evgeny Shvarov

If the intent is to wrap the statements in a new variable scope, why not simply extract the block in a private  method ?

0
Thomas Carroll · Jan 31, 2018

We want our code to be as accessible and readable as possible to as many people as possible. We always prefer whitespace formatting and curly brackets to arcane one line syntax. Someone who's not an Object Script expert will know what this does -

if a { 
   set b=a 
}

But there's no guarantee that a novice will necessarily know what these do -

if a set b=a
set:a b=a

Good code is readable and clever one liners will get in the way of that goal.

0
Fabian Haupt  Apr 27, 2018 to Thomas Carroll

"We want our code to be as accessible and readable as possible to as many people as possible. " Do we? ;) "We always prefer whitespace formatting and curly brackets to arcane one line syntax." -- I don't ;) Using arcane one liners shows that you are a superiour wizard. One should always use the short forms of commands to make code more terse and save space. If people want to read the code, they can use 'expand code' in studio (which expands the shortform commands to the full version). Note, there might be some sarcasm in here. People who know me, might also know that I'm only maybe 80% joking;)

0
Robert Cemper  Apr 27, 2018 to Fabian Haupt

you remind me the joke about hieroglyphs:
-  What's wrong with them?
- Nothing! Priests in old Egypt were reading the "book of death" like you would read a newspaper.
- You just have to learn the 'encryption'.

0
Joel Solon · Jan 31, 2018

The legacy versions of If and For commands don't use curly braces. But there are no legacy versions of While and Do/While, which require curly braces. So rather than trying to remember which commands use or don't use curly braces, just use curly braces all the time for these, as well as Try/Catch.

Regarding post-conditionals, it's a good habit to put parentheses around the condition, which allows you to put spaces in the condition.

This is valid syntax: set:a=4 y=10

This is valid syntax and more readable: set:(a = 4) y = 10

This is invalid syntax: set:a = 4 y = 10

0
Kishan Ravindran · Feb 1, 2018

I just went through the whole conversation on this and find it knowledgeable for me as i am beginner for cache. Now i am able to understand how to use them in with the standards and where i need to careful thanks for all. 

0
Evgeny Shvarov · Feb 1, 2018

Hi, colleagues!

The sense of the discussion I raised is to get Coding Guidelines for ObjectScript to let us cook the code which suits the goals and team guidelines the best. 

Thanks for a lot of bright thoughts and true experience!

0
Stephen Wilson · Apr 26, 2018

If I am modifying a routine I didn't originally write then I tend to stick to the coding style the original author adopted - even it is using legacy syntax. It is good practice not to mix old and new constructs within the same code block ie. don't mix curly braces with line-oriented syntax.  If you are completely re-writing something or writing new code then I prefer curly braces.

I would have liked to re-factor some of my code better but I found the 'use for' construct breaks when you try that. In this pseudocode, I was using the 'write' statement to write to a file on the file system. There is a condition that is checked to determine whether it gets written to file1 or file2.

// Run an extract based on arrayOfIds
for arrayIndex=1:1:arrayOfIds.Count() 
{
       // Continue to loop through array of you detect an errorStatus
      continue:errorStatus'=1
      open file1:("WNS":::):10
      // Inner-for 1
      use file1 for testIndex=1:1:testSetObj.TestCollection.Count()
       {
       }
       open file2:("WNS":::):10
       // Inner-for 2
       use file2 for testIndex=1:1:testSetObj.TestCollection.Count()
       {
       }
// end outter-for
0
Robert Cemper  Apr 26, 2018 to Stephen Wilson

I miss 3 things in this example:

  • There is a condition: I found no IF nor anything
  • at the end of the inner loop there is no CLOSE file1 7 /  CLOSE file2  for file1 or file2
  • OPEN file:("WNS": ......)   
    • W - Write is ok
    • S - Stream mode is OK
    • N - i s questionable. if you are not on VMS you override the file at each outer loop
      http://docs.intersystems.com/latest/csp/docbook/DocBook.UI.Page.cls?KEY=GIOD_rmsseqfiles#GIOD_rmsseqfiles_use
      New file. If the specified file does not exist, the system creates the file. If the specified file already exists, the system creates a new one with the same name. The status of the old file depends on which operating system you are using. On UNIX® and Windows, Caché deletes the old file. (Note that file locking should be used to prevent two concurrent processes using this parameter from overwriting the same file.)

      So to get all output you may append arrayIndex to filename to have unique file names

0
Stephen Wilson  Apr 27, 2018 to Robert Cemper

Thanks for the comments Robert. It's safe to assume there is an IF statement in each inner-for  to determine whether to do anything or not. Each object in arrayOfIds needs to be opened and a Boolean check is done against a property. I haven't had a problem with the 'N' parameter. The code appears to store 'write' records in process memory before writing everything to file after the outer-for. $ZSTORAGE has been buffed to accommodate this.

// Changes the max-limit on the process memory from the default 16384 Kilobytesset $ZS=49152

Unique file names have been achieved using the following assumptions:

1) The process will run no more than once a day
2) Append today's date in ODBC format using $zd(+$h,3) to the end of the filename.

The 'close' statement appears after the outer-for but I'm not sure if the curly braces implicitly does it anyway.

// Null check. Close if necessary. (May be implicitly closed by outter-for loop).if $GET(file1)'="" close file1if $GET(file2)'="" close file2
0
Robert Cemper  Apr 27, 2018 to Stephen Wilson

Thanks for the clarification.
It's sometimes hard to guess the picture if you miss some pixels.

I'd still recommend moving the CLOSE after each inner loop to have OPEN / CLOSE strictly paired

The implicit CLOSE happens only if you exit the partition. There's no class to act undercover. 
Curly braces just structure the code with no operational side effect.

0