How Many Parameters Should a Good Method Have?

Primary tabs

Hi Community!

Sometimes I meet a method which accepts 10+ parameters.

And often I need only the 8th parameter  to  pass. And I call the method something like:

do ##class(Some.Feature).Method(,,,,,,,"flag")

And I don't like this method when I call it like this cause, you know, often I just miss the number of commas and raise some other flag I wanted.

How do you avoid this situations? 

If you meet such a code, how do you call it and sure that you didn't miss the number of ","?

What is a good number of parameters in a method and f you need to pass more parameters in a method what do you do?

  • + 3
  • 1
  • 545
  • 23
  • 4

Answers

Hi,

Methods and call to methods, like any other code you write, need to be reviewed and tested.
When you type in studio you will see a tool-tip with the current parameter position in bold. 

Some times, when you need to pass a huge number of parameters to a function/method you might put there in a key=value array and simply pass this by ref.  

Yes. It's easy if you are in IDE.

But often you need to call this useful function from terminal...

It's really a matter of taste.

If you don't like the long list of params (especially with lousy documented methods) you can

#1) use 1 single param and pass a local array byRef .  and decode it yourself eg:

par("IPadrr")="127.0.0.1")
par("Port")=1972
par(Namespace")="SAMPLES"

.....

do  ##class(my.par).method(.par)

or

#2) use the traditional style you may know from Caché I/O Device Guide  having also just 1 parameter

do  ##class(my.pieces).method("/SERVER=127.0.0.1:/PORT=1972:/NAMESPACE=SAMPLES")

 

I personally prefer #2) as it gives you an embedded doc on your intentions.

One variation to #1 would be to  pass an object. It provides for readability (assuming property names are legible) and would allow for flexibility if you need to add a new parameter in the future (method signature doesn't need to change). 

I prefer objects as well especially when the amount of optionals are getting too large. I think when you start adding more and more optionals then it's time for a rethinking of what the function is really trying to accomplish.

Assuming you get the call string as the variable parameter then

for par="SERVER","PORT","NAMESPACE" set @par=$P($P(parameter,par_"=",2),":") zw @par
SERVER="127.0.0.1"
PORT=1972
NAMESPACE="SAMPLES"

Look at this

Class User.Test
{

ClassMethod Test(Args...)
{
  ZWRITE Args
}

ClassMethod Test2(Arg1, Arg2, Arg3, Arg4, Arg5, Arg6, Arg7, Arg8)
{
  ZWRITE Arg1
  ZWRITE Arg2
  ZWRITE Arg3
  ZWRITE Arg4
  ZWRITE Arg5
  ZWRITE Arg6
  ZWRITE Arg7
  ZWRITE Arg8
}

}

and let's call it

USER>do ##class(Test).Test(1,2,,,,,,8)  
Args=8
Args(1)=1
Args(2)=2
Args(8)=8

and second method

USER>do ##class(Test).Test2(1,2,,,,,,8) 
Arg1=1
Arg2=2
Arg8=8

And another way 

USER>set args=8 ;just the number of the latest one                      

USER>set args(8)="test"

and calls

USER>do ##class(Test).Test(args...)
Args=8
Args(8)="test"

USER>do ##class(Test).Test2(args...)
Arg8="test"

Yes, Dmitry.  Args... feature is neat, but you don't know what each parameter does so it's easy to miss the thing when you call the method, because the Args(index) should be a number, but not a meaningful parameter.

Some thoughts.

1. Do not use Hungarian notation.

2. Do not use ByRef params/dynamic objects/$lists/args... as an arguments. There are two exceptions:  you don't know the number of arguments or there's more than 10 arguments.

3. Method should not parse arguments, only validate arguments. If caller passed invalid input it's caller problem. Fail fast.

4. Mandatory arguments go first.

5. If several methods have similar signatures, the order of arguments should be the same.

6. Same logical arguments in different methods should have the same names.

7. Arguments should be consistent. If you pass primitives, pass primitives everywhere. If you pass dynamic objects, pass them everywhere.

8. Return types should also be consistent.

9. If it's object methods and you pass the same optional arguments in several methods extract these arguments as a property.
 

 

score

-1  #1 - disagree, see no valid reasoning

-2  #2 - strongly disagree

-1 #3 - disagree because of #2

+1 #4- agree

+1 #5 - agree

+1 #6 - agree . pls.send enhancement req. to engineering

+0 #7 - not clear about the message ? an oref is neither primitive nor dynamic

+0 #8 - miss imagination of inconsistent return types

+0 #9 - don't understand that message
 

1. Modern editors color variable based on scope. p/t in the beginning is unnecessary.

7. I mean if you develop an API all methods should accept either primitives or ByRefs or json. Not object arguments in one method, primitives in another, etc.

8. If you return %Status always return it. If you return $this always return self.

9. If your class has several instance methods and each has, let's say "debug" argument, remove this argument from methods and add "debug" as a class property.

Ah, got it.
I understand and agree on 7, 8, 9

for 1. I still wait for the "modern" editor. 
None of the existing ones could really convince me. But all are easier to handle than vi or X ^%  devil

Comments

Good question. Given our long history, we have a fair number of utility methods whose signatures grew organically and for which backwards compatibility goals have prevented us from doing significant cleanup. We're now in the process of reviewing the SQL utility methods and for the ones that really need as many parameters, are considering the use of an options string (aka compile flags), as is being used by some more modern infrastructure such as the work queue manager and/or just making separate specific methods rather than super-generic 10-argument ones. While we're at it, I'm eager to read suggestions or feedback on how others developing utility functions are dealing with this.

Guys, thanks for a lot of options! It looks like it is a relevant question ;)

Personally I feel that 3 parameters for a method are OK, but 4 is too much. But how much is for an average developer? Is there is a consensus about it?

If I know, that method will accept a lot of settings and not all of them are mandatory, I like the .args approach, which @Robert.Cemper mentioned and which, e.g. is introduced in e.g. in Database, Namespace and Web App management functions. E.g.:

set props("NameSpace")="Namespace"

set props("Enabled")=1

set props("DeepSeeEnabled")=1

Do ##class(Security.Applications).Create("AppName", .props)

It's readable and you don't count commas.

Also I'm curious if we can setup and pass a JSON that easily too? Would be great.

Also I'm curious if we can set up and pass a JSON that easily too? Would be great.

Certainly.

Class dc.test Extends %RegisteredObject
{

ClassMethod MethodTest(args As %DynamicObject)
{
 i $IsObject(args{
   args.%ToJSON(),!
   w:args.%IsDefined("arr"args.arr."2",!
 }else{
   "null",!
 }
}

ClassMethod Test()
{
  ;d ##class(dc.test).Test()
  ..MethodTest(),
    ..MethodTest({}),
    ..MethodTest({"arg1":($zts),"arg6":"hello","arr":["a",10,true,2.5674]})
}

}

Result:

USER>##class(dc.test).Test()
null
{}
{"arg1":"65136,19638.022","arg6":"hello","arr":["a",10,true,2.5674]}
1

I use Json when I can.  You can send a full book in one message if you wish to without creating a cluster

Hi. The "clean code" people would recommend just one parameter max, and better would be none! But I think that's going a bit far, and agree that 3, or maybe 4, maximum should be aimed for to keep things easy to understand when reading, though there may be exceptions.

The array passing is a good idea to reduce the number for normal routines, but I think the ideal for classes is using objects. If you are truly embracing objects, and self-documenting code, then new classes are usually needed and what used to be parameters become the setting of properties, like this:

table=##class(CMT.UI.Table).%New()
table.TopLine=10,table.BottomLine=21
table.HeaderFormat="Underline"
table.DefineQuery("CMT.UI.PatchSite:MyList")
table.AddColumn(3,,"BOLD")
...etc.

table.Display()

It works well in some cases, but I have to admit that there is a tendency for the number of classes to get a bit silly if you take it to the extreme. Sometimes simple code is best.  :-)     / Mike

UPDATE: Oh, this technique has been mentioned already, either way this sample shows why to use it.

That's gonna be quite the bump, but it's worth mentioning. Since the adoption of %Dynamic classes, you can now provide a JSON object to methods, this way you get a syntax that ressembles POJOs (Plain Old JavaScript Objects):

ClassMethod GreetAndAsk(data As %DynamicObject) As %String
{
    return $$$FormatText("Hello %1! %2", data.name, data.message)
}

// Omitted for brevity.
write ..GreatAndAsk({
  "name": "Rubens",
  "message": "How are you?"
})

This is really useful if you must provide a method with several parameters that have some sort of link between them, here's a real use-case:

/// This method implements a full-sized configuration on how to handle multipart uploads.
ClassMethod TestPOSTMultipartFileUpload() As %Status
{
  set location = %frontier.Data.Workspace_"/fixtures/uploads/multiple"
  set destination = (location_"/:KEY/:FILE_NAME:EXTENSION")

  // 512 KB
  set maxFileSize = (1024**2/0.5)

  return %frontier.Files.Upload({
    "hooks": {
      "onItemUploadSuccess": "Frontier.UnitTest.Router:WriteResultToProcessGlobal",
      "onItemUploadError": "Frontier.UnitTest.Router:WriteErrorToProcessGlobal",
      "onComplete": "Frontier.UnitTest.Router:WriteResultSummaryToProcessGlobal"
    },
    "filters": {
      "verbose": true,
      "maxFileSize": { "value": (maxFileSize), "errorTemplate": "The file :FILE_NAME exceeded the max of :VALUE bytes." },
      "extensions": ["cls", "md", "txt", "pdf"]
    },
    "destinations": {
      "file_a": { "path": (destination) },
      "file_b": { "path": (destination) },
      "file_c": { "path": (destination), "optional": true },
      "file_d": { "path": (destination), "optional": true },
      "files": {
        "path": (location_"/files/:INDEX/:FILE_NAME:EXTENSION"),
        "slots": 3,
        "filters": { "maxFileSize": 500000000 }
      }
    }
  })
}

The only real solution is support of named arguments (like Python)

They are on my ISC wish-list for many years ;-)

To incorporate in COS-native is a challenge, but in Methods it shouldn't be so hard.

Herman 

I think you have it in COS since Strings became almost endless.
Par #2) of my answer showed it:

the traditional style you may know from Caché I/O Device Guide  having also just 1 parameter

do  ##class(my.pieces).method("/SERVER=127.0.0.1:/PORT=1972:/NAMESPACE=SAMPLES")

and the method end:

ClassMethod method(parameter as %String(MAXLEN="") {
   for par="SERVER","PORT","NAMESPACE" 
   set @par=$P($P(parameter,par_"=",2),":") 
   zw @par 
   #; now do something ...   
}
SERVER="127.0.0.1" 
PORT=1972 
NAMESPACE="SAMPLES"

Robert

You're right it works, but ... you loose the method signature/contract.

With a Method such as:

Method(Name as %String, Address as Address, Age as %Integer)

The compiler can enforce the right named arguments:

Do Method(Age=32) -> OK

Do Method(Car="Maserati") -> not OK (except for the car of course ;-))

Robert,

As to (simulated) named arguments approach:

(+): Good readability.

(-):  We should be pretty sure that values would not contain separators, neither "=" nor ":", although screening of them is not a very hard problem ("\=", "\:", "\\"). 

(-): Handling binary data (e.g., $list or $bit) seems to be more serious problem. In this case it seems that one argument structured as $lb($lb("name1",value1),...$lb("nameN",valueN)) is better option.

Alexey,

you are perfectly correct. My intention was to demonstrate that the ancient approach of 
OPEN and USE command parameters dating from the previous millennium is still valid.

wink
$LB() is definitely the more general and flexible solution. 

yes