-
Notifications
You must be signed in to change notification settings - Fork 16
Conversation
prjelesi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Fix Function Scope for Excel Worksheet and Row
Problem:
Set-ColumnColor uses $ws and $row variables that are not passed in, so the function cannot know which worksheet or row to operate on.
How to fix:
Update the function to accept both worksheet and row as parameters, and update the call in New-Worksheet:
PowerShell
Function Set-ColumnColor {
param(
[Parameter(Mandatory = $true)] [object]$ws,
[Parameter(Mandatory = $true)] [int]$row,
[Parameter(Mandatory = $true)] [int]$startColumn,
[Parameter(Mandatory = $true)] [string]$cellValPositive,
[Parameter(Mandatory = $true)] [string]$cellValNegative
)
$colCount = $ws.Dimension.End.Column
for ($col = $startColumn; $col -le $colCount; $col++) {
$colLetter = [OfficeOpenXml.ExcelCellAddress]::GetColumnLetter($col)
$cell = $ws.Cells["$colLetter$row"]
if ($cell.Value -eq $cellValPositive) {
$cell.Style.Fill.PatternType = [OfficeOpenXml.Style.ExcelFillStyle]::Solid
$cell.Style.Fill.BackgroundColor.SetColor([System.Drawing.Color]::LightGreen)
}
elseif ($cell.Value -eq $cellValNegative) {
$cell.Style.Fill.PatternType = [OfficeOpenXml.Style.ExcelFillStyle]::Solid
$cell.Style.Fill.BackgroundColor.SetColor([System.Drawing.Color]::LightCoral)
}
}
}
Then, in your New-Worksheet function, change the call to:
PowerShell
for ($row = 2; $row -le ($reportData.Count + 1); $row++) {
If ($startColumnNumber) {
Set-ColumnColor -ws $ws -row $row -startColumn $startColumnNumber -cellValPositive $cellValPositive -cellValNegative $cellValNegative
}
}
2. Check ImportExcel Module Before Use
Problem:
If ImportExcel is not installed, the script will fail.
How to fix:
Add to the top of your script:
PowerShell
if (-not (Get-Module -ListAvailable -Name ImportExcel)) {
Write-Error "ImportExcel module is required. Install it with: Install-Module -Name ImportExcel"
exit 1
}
3. Add Error Handling for Input Files
Problem:
No error handling for missing or corrupt input files.
How to fix:
Wrap file reading in a try/catch block, e.g.:
PowerShell
try {
$rawdata = Get-Content $path | ConvertFrom-Json -Depth 10
} catch {
Write-Error "Failed to read or parse JSON from $path: $_"
exit 1
}
4. SKUs String Concatenation (Recommended for Correct Output)
Problem:
Currently, $implementedSkus is concatenated as a string in a loop, which is fragile.
How to fix:
Build an array and join:
PowerShell
$skuList = @()
foreach ($sku in $item.ImplementedSkus) {
switch ($resourceType) {
"microsoft.compute/virtualmachines" { $skuList += $sku.vmSize }
default { $skuList += $sku.name }
}
}
$implementedSkus = $skuList.Count -gt 0 ? ($skuList -join ", ") : "N/A"
Summary:
Pass all variables explicitly to functions (especially $ws and $row).
Check for module dependencies before using them.
Add error handling to file operations.
Use arrays for string concatenation in loops, then join.
Making these changes is necessary for the script to work without runtime errors and to produce the intended Excel output.
prjelesi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Add handling of cost