A Macro function for validating countries - Unexpected "hole" in the error catching

[SasStatistics]

I have written a function to validate wether a country is in a specific list of the nordic countries (SE DK NO FI). 

So for example, 

%put test = %ValidateCountry(se, no fi);  #returns the logical 1. 

The whole function looks like this (For the variables, b stands for boolean and m stands for macro. Example: countrym indicates that the variable is a macro variable).

%macro ValidateCountry(countrym /* Choose SE, DK, NO or FI. */, 
							countrylistm /* A used defined list contaning a combination of SE, DK, NO, FI. */
							 ) /minoperator mindelimiter=' ';

/*--------------------------------------------------------------*/
/*Help Functionality*/
/*--------------------------------------------------------------*/
	%if %upcase(&countrym.)= HELP or &countrym.=? %then %do;
	    %put WARNING-************************************************************************;
	    %put WARNING-*   Description: Validate a country, the function is case-insensitive. *;
	    %put WARNING-*   Syntax: ValidateCountry(countrym, countrylistm, caseb).            *;
	    %put WARNING-*   Parameters:                                                        *;
	    %put WARNING-*     countrym: One of SE, DK, NO, FI.                                 *;
	    %put WARNING-*     countrylistm: a combination of SE, DK, NO, FI.                   *;
	    %put WARNING-*    ----------------------------------------------------------------  *;
	    %put WARNING-*      Example 1: ValidateCountry(SE, SE NO) returns 1.                *;
	    %put WARNING-*      Example 2: ValidateCountry(nO, sE NO Fi) returns 1.             *;
	    %put WARNING-*      Example 3: ValidateCountry(SE, SE USA) returns error.           *;
	    %put WARNING-************************************************************************;
	    %return;
	%end;

/*--------------------------------------------------------------*/
/*Validation*/
/*--------------------------------------------------------------*/
	%local countrynotvalidbm countrylistnotvalidbm i nextm listm validbm;

	%let countrynotvalidbm = 0;   /* Initializing the variables used below */
	%let countrylistnotvalidbm = 0; 
	%let validbm = 0; 

	%let listm=%qupcase(SE DK NO FI);

	%if not (%qupcase(&countrym) in &listm) %then %do;
		%let countrynotvalidbm=1;
		%put ERROR: COUNTRY value "&countrym." must be ONE of the values: &listm .;
	%end;

	%do i=1 %to %sysfunc(countw(&countrylistm.));
		%let nextm=%qscan(&countrylistm.,&i,%str( ));

		%if not (%qupcase(&nextm) in &listm) %then %do;
		   %let countrylistnotvalidbm=1;
		   %put ERROR: COUNTRYLIST value "&nextm" is not among the values: &listm. ;
		%end;
	%end;

/*--------------------------------------------------------------*/
/*Main Body*/
/*--------------------------------------------------------------*/
	%if (&countrynotvalidbm. = 1 or &countrylistnotvalidbm. = 1) %then %do ; 
		%put ERROR: The function was not run due to errors. ; 
	%end;
 
	%else %do;
		%if %upcase(&countrym.) in %upcase(&countrylistm.) %then %do; 
	 		%let validbm= 1; 
		%end; 
	%end; 

	&validbm. ; /*1 if the country is in the valid list */

%mend; 

Now, running code below yields an error. I was hoping that the validation part of the function would catch it, but it does not. Error messages appearing are: 

%put test = %ValidateCountry(,se no); 

/* The following error appears */
ERROR: Operand missing for IN operator in argument to %EVAL function.
ERROR: The macro VALIDATECOUNTRY will stop executing.


%put test = %ValidateCountry(se , );

/* The following error appears */
ERROR: The function COUNTW referenced by the %SYSFUNC or %QSYSFUNC macro function has too few arguments.
ERROR: A character operand was found in the %EVAL function or %IF condition where a numeric operand is required. The condition was: 
%sysfunc(countw(&countrylistm.)) 
ERROR: The %TO value of the %DO I loop is invalid.
ERROR: The macro VALIDATECOUNTRY will stop executing.

I don't know how to solve this "hole" in the error handling. All advice much appreciated. 

[SASJedi]

You will find that the macro IN operator will fail and throw an error if a NULL value is presented to the operator on either side. The way your macro is constructed, you are expecting either a single value in countrym or a list in countrylistm. This means that one of those values will always be NULL. In the body of the macro, both values are always tested with an IN operator, so the IN operator is always exposed to a NULL value. Hence the errors you are seeing. 

May the SAS be with you!

Mark

[PhilC]

Do you have parentheses around your list after your in operator? Submitting to you with out testing, I think this is what you are lacking.

	%else %do;
		%if %upcase(&countrym.) in ( %upcase(&countrylistm.) ) %then %do; 
                                         /**/                       /**/
	 		%let validbm= 1; 
		%end; 

[Quentin]

For testing for blank macro variables, I would recommend:

https://support.sas.com/resources/papers/proceedings09/022-2009.pdf

Which will allow you to do stuff like:

%if %isblank(countrym) %then %do;
  %put ERROR: CountryM parameter is required. ;
%end;

[Tom]

Issues:

Test for empty values.

Include the delimiter in the call to COUNTW().

Use macro quoting consistently.  The last IN operation did not macro quote the value being tested.

Do not emit the semi-colon as part of the macro results.

And on formatting :

- do not include physical tab characters into code files.  It causes things like this when opened by programs that are using different tab stops than you thought.

- place continuation characters on multi-line statements at the START of the line, not the END of the line so that humans scanning the file can see them.

- Make the parameter names in your error messages match the parameter names in the macro.

%macro ValidateCountry
(countrym       /* Choose SE, DK, NO or FI. */
,countrylistm   /* A used defined list contaning a combination of SE, DK, NO, FI. */
) /minoperator mindelimiter=' ';

/*--------------------------------------------------------------*/
/*Help Functionality*/
/*--------------------------------------------------------------*/
  %if %upcase(&countrym.)= HELP or &countrym.=? %then %do;
      %put WARNING-************************************************************************;
      %put WARNING-*   Description: Validate a country, the function is case-insensitive. *;
      %put WARNING-*   Syntax: ValidateCountry(countrym, countrylistm, caseb).            *;
      %put WARNING-*   Parameters:                                                        *;
      %put WARNING-*     countrym: One of SE, DK, NO, FI.                                 *;
      %put WARNING-*     countrylistm: a combination of SE, DK, NO, FI.                   *;
      %put WARNING-*    ----------------------------------------------------------------  *;
      %put WARNING-*      Example 1: ValidateCountry(SE, SE NO) returns 1.                *;
      %put WARNING-*      Example 2: ValidateCountry(nO, sE NO Fi) returns 1.             *;
      %put WARNING-*      Example 3: ValidateCountry(SE, SE USA) returns error.           *;
      %put WARNING-************************************************************************;
      %return;
  %end;

/*--------------------------------------------------------------*/
/*Validation*/
/*--------------------------------------------------------------*/
  %local countrynotvalidbm countrylistnotvalidbm i nextm listm validbm;

  %let countrynotvalidbm = 0;   /* Initializing the variables used below */
  %let countrylistnotvalidbm = 0; 
  %let validbm = 0; 

  %let listm=%qupcase(SE DK NO FI);

  %if %length(&countrym) %then %if not (%qupcase(&countrym) in &listm) %then %do;
    %let countrynotvalidbm=1;
    %put ERROR: COUNTRYM value "&countrym." must be ONE of the values: &listm .;
  %end;
  %else %do;
    %let countrynotvalidbm=1;
    %put ERROR: COUNTRYM value cannot be empty. ;
  %end;

  %let i=%sysfunc(countw(&countrylistm.,%str( )));
  %if &i %then %do i=1 %to &i;
    %let nextm=%qscan(&countrylistm.,&i,%str( ));

    %if not (%qupcase(&nextm) in &listm) %then %do;
       %let countrylistnotvalidbm=1;
       %put ERROR: COUNTRYLISTM value "&nextm" is not among the values: &listm. ;
    %end;
  %end;
  %else %do;
       %let countrylistnotvalidbm=1;
       %put ERROR: COUNTRYLISTM value cannot be empty.;
  %end;

/*--------------------------------------------------------------*/
/*Main Body*/
/*--------------------------------------------------------------*/
  %if (&countrynotvalidbm. = 1 or &countrylistnotvalidbm. = 1) %then %do ; 
    %put ERROR: The function was not run due to errors. ; 
  %end;
 
  %else %do;
    %if %qupcase(&countrym.) in %upcase(&countrylistm.) %then %do; 
       %let validbm= 1; 
    %end; 
  %end; 

  &validbm.  /*1 if the country is in the valid list */

%mend ValidateCountry;

[Quentin]

As an aside, personally I don't think I'd use macro quoting at all here.

You don't need quoting (or even upcasing) here because you hard-coded a string that doesn't have any symbols that need to have their meaning hidden:

%let listm=%qupcase(SE DK NO FI)

I guess some folks follow the guideline of macro quoting every user parameter, in case a user enters a value like OR, NE, etc.

But I tend to only macro quote when there is a likely need (such as US state abbreviations), rather than to protect against any crazy value somebody might enter.  Of course I sometimes pay the price for that, depending on how widely a macro will be used, because I'm often surprised that people put & symbols in file names and other fun stuff. 

[SasStatistics]

Thank you very much @Tom  . 

"- place continuation characters on multi-line statements at the START of the line, not the END of the line so that humans scanning the file can see them." What do you mean by this? 

Running the code you provided, I got the following obviously wrong (Non desirable) answers when running the code (indicating the function is wrong): 

1. 
%let test=%ValidateCountry(,SE DK);  /* Should be COUNTRYM value cannot be empty. */ 

ERROR: Operand missing for IN operator in argument to %EVAL function.
ERROR: The macro VALIDATECOUNTRY will stop executing. 

2. 
%let test=%ValidateCountry(SE, ); 

ERROR: COUNTRYM value cannot be empty.
ERROR: COUNTRYLISTM value cannot be empty.
ERROR: The function was not run due to errors.

3. 
%put test = %ValidateCountry(sE, UK SE); /* OK */

ERROR: COUNTRYM value cannot be empty.
ERROR: COUNTRYLISTM value "UK" is not among the values: SE DK NO FI
ERROR: The function was not run due to errors.

4. 
%put test = %ValidateCountry(se, UK USA SE); /* OK */

ERROR: COUNTRYM value cannot be empty.
ERROR: COUNTRYLISTM value "UK" is not among the values: SE DK NO FI
ERROR: COUNTRYLISTM value "USA" is not among the values: SE DK NO FI
ERROR: The function was not run due to errors.

5. 
%put test = %ValidateCountry(se, no); /* OK */

ERROR: COUNTRYM value cannot be empty.
ERROR: The function was not run due to errors.

For the code line: 

%if %length(&countrym)

does it actually mean: 

%if %length(&countrym) > 0 ?

Thanks alot. 

[Tom · Accepted solution]

Issue was mismatched %IF/%ELSE.

Try this update:

  %if not %length(&countrym) %then %do;
    %let countrynotvalidbm=1;
    %put ERROR: COUNTRYM value cannot be empty. ;
  %end;
  %else %if not (%qupcase(&countrym) in &listm) %then %do;
      %let countrynotvalidbm=1;
      %put ERROR: COUNTRYM value "&countrym." must be ONE of the values: &listm .;
  %end;

SAS will treat 0 (or missing) as FALSE and any other number as TRUE.  So no need to add the extra characters.

[Tom]

You need to provide what logic you want to do as this case appears to be working as your original macro intended.

929   %put test = %ValidateCountry(se, UK USA SE);
ERROR: COUNTRYLISTM value "UK" is not among the values: SE DK NO FI
ERROR: COUNTRYLISTM value "USA" is not among the values: SE DK NO FI
ERROR: The function was not run due to errors.
test = 0

Those two values are clearly NOT in the list of valid values your macro defined.

If you want to use COUNTRYLISTM as an OVERRIDE for the hardcoded list of valid value why did you include that test to begin with?  That is much easier to code.

%let listm=%qupcase(SE DK NO FI);
%if %length(&countrylistm) %then %let listm=%qupcase(&countrylistm);

And now there is no need to "test" COUNTRYLISTM.

[Astounding]

While I can't test it (or I would), I believe you can simplify tremendously and avoid the complications ... along the lines of:

%let found = %sysfunc(indexw(&countrylistm, &countrym)) > 0;

You still need to apply %upcase along the way, but you don't need a macro IN operator.

[Quentin]

Good point.  It took so long for SAS to get the macro IN operator working, and the first implementations were so confusing, that I've never bothered to learn how to use it.  It still feels clunky to have to specify minoperator and mindelimiter.

[PhilC]

Is there anything wrong with using FIND()?

%let _=; /*@jimbarbour*/

&_ %macro _ValidateCountry
    (countrym       /* Choose SE, DK, NO or FI. */
    ,countrylistm   /* A used defined list contaning a combination of SE, DK, NO, FI. */
    ) /minoperator mindelimiter=' ';
  %let valid=%eval( %sysfunc(find(SE DK NO FI,&countrym,i)) > 0);
  %let return=%eval( (%sysfunc(find(&countrylistm,&countrym,i))) > 0 );
  %eval( &return * &valid );
%MEND;

%put %_ValidateCountry(SE, SE NO) ;      
%put %_ValidateCountry(nO, sE NO Fi);   
%put %_ValidateCountry(SE, SE USA); 
%put %_ValidateCountry(,se no); 
%put %_ValidateCountry(se , ); 

@jimbarbour 

[Astounding]

@PhilC , there would be a problem using FIND.  It doesn't look for words.  So if the (invalid) country code is O, for example, FIND would find it within:

SE NO FI

[jimbarbour]

@PhilC,

I think that FINDW might work better.

With FINDW, you don't need MINOPERATOR or MINDELIMITER anymore.

An interesting thing that I sometimes do in macros is to assign a default while still allowing for positional parameters:

%let _=; /*@jimbarbour*/

&_ %macro _ValidateCountry
    (countrym       /* Choose SE, DK, NO or FI. */
    ,countrylistm   /* A used defined list contaning a combination of SE, DK, NO, FI. */
    );
/*	%PUT	&Nte1  &=countrym  &=countrylistm;*/
	%IF	%BQUOTE()	=	%BQUOTE(&CountryListM)	%THEN
		%LET	CountryListM	=	SE DK NO FI;
/*	%PUT	&Nte1  &=countrym  &=countrylistm;*/
  %eval(%sysfunc(findw(&countrylistm,&countrym,,SI)) > 0);
%MEND;

OPTIONS	NOSOURCE;
%put NOTE: %_ValidateCountry(SE, SE NO) ;      
%put NOTE- %_ValidateCountry(nO, sE NO Fi);   
%put NOTE- %_ValidateCountry(SE, SE USA); 
%put NOTE- %_ValidateCountry(,se no); 
%put NOTE- %_ValidateCountry(se , ); 
%put NOTE- %_ValidateCountry( , NE); 
%put NOTE- %_ValidateCountry(NO , NE); 
%put NOTE- %_ValidateCountry(o , ); 
OPTIONS	SOURCE;

In the above, the first %BQUOTE is not really necessary, but sometimes is helpful to me just in terms of readability.  The second %BQUOTE is actually doing something.  Try running the code with and without the %BQUOTE, but uncomment the %PUT

%PUT	&Nte1  &=countrym  &=countrylistm;

before and after the %IF statement and run with parameters (NO, NE)

%put NOTE- %_ValidateCountry(NO , NE); 

Jim