SOLVED

Return command is not coming out of function and not returning value

Copper Contributor

I am using below code

1) To verify whether given input text is in JSON format

then

2) User acceptance on the file selected as per its version.

 

Means, if i give wrong JSON format in the input file name, it will ask for a correct formatted file so that I will verify the JSON code in file, correct it and then input again. Then it will verify the version of the JSON file and asks for user acceptance. Post user acceptance, return value otherwise ask for alternate file with required version.

 

But, lets say if i took 'n' iterations to get correct JSON format, post giving correct formatted JSON as input, user acceptance is also iterating 'n' times even though it is accepted initial time itself. Means, 'return' is not coming out of the function (which should actually come out of the function as per my knowledge). Please correct me if my assumption about 'return' is wrong or correct the code.

 

Note: By using another function for JSON validation I got the issue fixed but I don't want to follow that process.

 

Code:


function Cfile
{
param([String]$title)
try
{
$filepath = Read-host "Enter the path of the file"
$JSONformat = gc $filepath | ConvertFrom-Json -ErrorAction Stop;
Write-Host "Valid JSON format" -ForegroundColor Green
}
catch
{
Write-Host "Provided text is not a valid JSON string" -ForegroundColor Red
Cfile -title "Select Correct Config file"
}

Write-Host "Entered baseline version is" $JSONformat.version
$confirm = Read-Host "Is that correct?? (Y/N)"
Switch($confirm)
{
Y{
$done = "Good job"
Break
}
Default{
Write-Host "Select Correct Config file"
cfile -title "Select Correct Config file"
}
}
Return($Done)
}

$Required = Cfile "Select required JSON"

 

 

Input JSON file (correct format):

{
"version": 87,
"TestingJSON": [
{
"type": "string",
"category": "crytical"
}
]
}

 

Note: To give wrong JSON format as input, may be you can remove any Comma, brace, quotation .. etc from above file (if you are not much familiar about JSON.

3 Replies
best response confirmed by SBVRaja (Copper Contributor)
Solution

@SBVRaja I several gotchas which may explain the behavior you're seeing...

 

  • Using recursive calls to CFile is probably not the best approach, especially from inside the catch {} block.
  • The $Done variable is never properly initialized, and the only possible return value is "Good job" for $Required.
  • I might recommend moving any statements referencing $JSONformat inside your try {} block.
  • Your $title argument to the CFile function is never used.
  • If possible, avoid using Write-Host; and instead try using either Write-Verbose, Write-Output, Write-Information, Write-Warning, etc.
  • Since your Switch() statement only handles 2 conditions, an if() {} else {} block might be cleaner.
  • Return($Done) is mostly equivalent to: $Done; Return; Since this is the last statement in your function, Return is really unnecessary.

That said, here's one possible re-write:

 

function Cfile
{
	[cmdletbinding()]
	param([string]$title = 'Enter the path of the file')
	
	begin {
		$done = 'Bad job'
	}
	
	process {
		Do {
			try {
				$filepath = Read-host $title
				$JSONformat = (get-content -Path $filepath -ErrorAction Stop | ConvertFrom-Json -ErrorAction Stop)
				Write-Verbose "Valid JSON format`nEntered baseline version is: $($JSONformat.version)"
			} catch {
				Write-Warning 'Missing file, or provided text is not a valid JSON string'
			}

			$confirm = Read-Host 'Is that correct?? (Y/N)'
			if ('Y' -eq $confirm) {
				$done = 'Good job'
			} else {
				$title = 'Select Correct Config file'
			}
		} Until ('Bad job' -ne $done)
	
		$done
	}
	
	end {}
}

$Required = Cfile -Verbose
Write-Output $Required

 

Hope this helps.

@JRCigars Thanks for looking into this and your time JRCigars. I really appreciate that. My actual problem here is not to find alternate solutions (As mentioned, I already had an alternate solution and I also believe there are many other solutions too just like Do-While you used which is also a great approach which I didn't think). But my actual cause here is to understand the behavior of 'Return' and put a '?' at where ever I don't have a proper answer
Considering this, here are my responses to your 7 points with '?' .. :)
1) Using recursive calls to CFile is probably not the best approach, especially from inside the catch {} block.
Raja: Understood. I may agree with you on this. But WHY?
2) The $Done variable is never properly initialized, and the only possible return value is "Good job" for $Required.
Raja: I don't think it is a big issue (in fact not a issue at all). In fact, the code that i gave is very simplified version of actual bulk code. These $Done, $title, the function CFile itself are all its simplified versions.
3) I might recommend moving any statements referencing $JSONformat inside your try {} block
Raja: I completely agree. This is actually simplifying the issue completely.
4) Your $title argument to the CFile function is never used.
Raja: I don't think its of big issue (in fact not a issue at all). In actual code, I generally use this variable to display while reading $filepath in try{} block as this function 'Cfile' i use for multi purpose. In fact, the code that i gave is very simplified version of actual bulk code. These $Done, $title, the function CFile itself are all its simplified versions.
5) If possible, avoid using Write-Host; and instead try using either Write-Verbose, Write-Output, Write-Information, Write-Warning, etc.
Raja: I would consider it. I will study this alternate approach
6) Since your Switch() statement only handles 2 conditions, an if() {} else {} block might be cleaner
Raja: I agree. But in actual code, there are 3 more conditions that it actually verifies.
7) Return($Done) is mostly equivalent to: $Done; Return;
Raja: I agree. But with the use of Return why the engine is not moving out of function??

Regarding 7) I suspect the main reason that Return doesn't exit out of the function is because of 1). Without seeing your actual function code, it's hard to tell. Examine all your code execution paths, and ensure your real function has a legitimate way to exit gracefully -- rather than going deeper into CFile with recursion. Given your simplified version posted here, it might be that "catch { CFile }" is the actual culprit why it never returns.
1 best response

Accepted Solutions
best response confirmed by SBVRaja (Copper Contributor)
Solution

@SBVRaja I several gotchas which may explain the behavior you're seeing...

 

  • Using recursive calls to CFile is probably not the best approach, especially from inside the catch {} block.
  • The $Done variable is never properly initialized, and the only possible return value is "Good job" for $Required.
  • I might recommend moving any statements referencing $JSONformat inside your try {} block.
  • Your $title argument to the CFile function is never used.
  • If possible, avoid using Write-Host; and instead try using either Write-Verbose, Write-Output, Write-Information, Write-Warning, etc.
  • Since your Switch() statement only handles 2 conditions, an if() {} else {} block might be cleaner.
  • Return($Done) is mostly equivalent to: $Done; Return; Since this is the last statement in your function, Return is really unnecessary.

That said, here's one possible re-write:

 

function Cfile
{
	[cmdletbinding()]
	param([string]$title = 'Enter the path of the file')
	
	begin {
		$done = 'Bad job'
	}
	
	process {
		Do {
			try {
				$filepath = Read-host $title
				$JSONformat = (get-content -Path $filepath -ErrorAction Stop | ConvertFrom-Json -ErrorAction Stop)
				Write-Verbose "Valid JSON format`nEntered baseline version is: $($JSONformat.version)"
			} catch {
				Write-Warning 'Missing file, or provided text is not a valid JSON string'
			}

			$confirm = Read-Host 'Is that correct?? (Y/N)'
			if ('Y' -eq $confirm) {
				$done = 'Good job'
			} else {
				$title = 'Select Correct Config file'
			}
		} Until ('Bad job' -ne $done)
	
		$done
	}
	
	end {}
}

$Required = Cfile -Verbose
Write-Output $Required

 

Hope this helps.

View solution in original post