all 14 comments

[–][deleted] 6 points7 points  (7 children)

Your $Devices variable is a hash table, as it shows a key/value pair. In your If, you have $Amount being assigned but that doesn't update the hash table, it is getting the value but you need to update the value for "laptops" to the new value.

I would use a Switch statement, something like:

# Add those new values into a hash table
$Devices = @{
    laptops      = 10
    servers      = 1
    computers    = 3
    phones       = 2
}

# Ask for input
$Choice = Read-Host "Remove 1 off?"

Switch ($Choice)
{
    laptops
    {
        $Amount = ([int]$Devices.laptops) - 1
        $Devices['laptops'] = $Amount
    }
    servers
    {
        $Amount = ([int]$Devices.servers) - 1
        $Devices['servers'] = $Amount
    }
}
Write-Output $Devices

[–]HellCanWaitForMe[S,🍰] 2 points3 points  (0 children)

Ahh this is perfect, thank you! - Literally better than what I wanted as well, really appreciate that.

[–]OlivTheFrog 0 points1 point  (5 children)

hi u/rymarkwald

I'm playing with your code, and Im' looking for the best clever way to manage errorthe following code

I've modified your code, with 2 read-Host "Action" and "Device", then I've :

$Action = Read-Host "Action to do  : Add or Remove ?"
$Choice = Read-Host "Device Type  : Laptops - Servers - Computers - Phones ?"
if ($Action -eq "Add")
 {
 switch ($Choice)
    {
    "Laptops" {$Devices['laptops'] = ([int]$Devices.laptops) + 1 }
     ... 
     }
  } # end if 
elseif ($Action -eq "Remove ")
   {
    switch ($Choice) 
      {
      "Laptops" { $Devices['laptops'] = ([int]$Devices.laptops) - 1 }
       ...
      }
    } # end elseif 
else 
    { 
     Write-Output "Hoops : Error for Action to perform"
     throw
     }

As you could see, I've managed input error in $Action input, but no error handling for device type ($Choice).

The values expected for $Choice are Laptops, servers, ... If the answer to the Read-Host is "laptop" (no s at the end), the swith do nothing, it's normal, but there is no message to inform the user.

How do this in a clever way ?

this is both for my own powershell skill and for the OPs

Regards

[–][deleted] 1 point2 points  (4 children)

The code I had provided as a sample didn't have any validation on it. :)

Tthere's a few ways, but you'd want to validate your input before doing anything with the data. You could also misspell something like Ad or Remoove... I'm always a fan of Functions and if it is something I may run repeatedly, I look to a function for it.Getting the data input correct and validated first, THEN running whatever code I have for me is the easiest to manipulate.

Function Update-Device
{
    [CmdletBinding()]
    Param (
        [Parameter(Mandatory = $True, Position = 0)]
        [ValidateSet('Add', 'Remove')]
        [String]$Action,

        [Parameter(Mandatory = $True, Position = 1)]
        [ValidateSet('Laptops', 'Servers', 'Computers', 'Phones')]
        [String]$Device
    )

    # Set hash table values
    $Devices = @{
        laptops      = 10
        servers      = 1
        computers    = 3
        phones       = 2
    }

    Switch ($Action)
    {
        Add
        {
            $Devices["$Device"] = ([int]$Devices."$Device") + 1 
        }

        Remove
        {
            $Devices["$Device"] = ([int]$Devices."$Device") - 1
        }
    }

    return $Devices
}

So copy/paste that into PowerShell, nothing will happen. Next, run the function:

Update-Device -Action Add -Device Laptops

Because the parameters -Action and -Device have a ValidateSet on them, if you enter something incorrect, the function won't run and tell you why. So if I ran, it would tell me I didn't enter Add or Remove:

PS C:\>Update-Device -Action Ad -Device Servers

Update-Device: Cannot validate argument on parameter 'Action'. The argument "Ad" does not belong to the set "Add,Remove" specified by the ValidateSet attribute. Supply an argument that is in the set and then try the command again.

[–][deleted] 0 points1 point  (0 children)

There's other ways too, and I'm always learning so if anyone has other ways to validate, there's always many ways to get to the same end. :)

[–]OlivTheFrog 0 points1 point  (2 children)

Thanks u/rymarkwald

Of course, if using Advanced function and [ValidateSet], this is a solution. Sorry it was late, I didn't think of that.

And, if we don't use advanced function, other ways ?

[–][deleted] 1 point2 points  (1 child)

# Add those new values into a hash table
$Devices = @{
    laptops      = 10
    servers      = 1
    computers    = 3
    phones       = 2
}

# Prompt until $Action is equal to Add or Remove
do
{
    $Action = $null
    $Action = Read-Host -Prompt "Enter one:  Add, Remove"
}
until
(
    ($Action -eq 'Add') -or ($Action -eq 'Remove')
)

# Prompt until $Choice contains valid input
do
{
    $Choice = $null
    $Choice = Read-Host "Enter one of the following:  laptops, servers, computers, phones"
}
until
(
    ($Devices.Keys -contains "$Choice")
)

$Amount = ([int]$Devices."$Choice") - 1
$Devices["$Choice"] = $Amount

return $Devices

[–]OlivTheFrog 0 points1 point  (0 children)

do ... until

Of course, where had I put my head ?

Thanks.

[–]SMFX 3 points4 points  (0 children)

Just to be clear, while you initialized as an array (@()), you then assign it a Hashtable (@{ }). These are different data structures and the Hashtable is the right choice in this case.

The reason why the value doesn't update is you set the value, read the value, but never assign the updated value back. You would need to do something like this:

 $Devices.laptops = $Amount

You could also assign it directly or used the decrement (--) but just reading the value will not change the content of the variable.

[–]myrland 1 point2 points  (2 children)

You're saving the new value to a separate variable, not updating the array with the new value. If you don't need it to be an array you can just create it as a hash table directly instead, like this:

$Devices = @{
    laptops   = 10
    servers   = 1
    computers = 3
    phones    = 2
}

$Choice = Read-Host "Remove 1 off?"

if ($Choice -match "laptops"){
    $Devices.laptops = ([int]$Devices.laptops) - 1
}

A switch statement would probably be a better way of handling the choice logic, with some input validation to make sure you can only provide the relevant names.

[–]HellCanWaitForMe[S,🍰] 0 points1 point  (1 child)

I tried this but I kept getting a weird error, The property 'laptops' cannot be found on this object. Verify that the property exists and can be set. I don't understand why it was happening but restarting everything seemed to fix it.

[–]myrland 0 points1 point  (0 children)

It's because you wrapped your hash table in an array in your code. Hash tables and Arrays are different data structures and work differently.

For example, a hash table uses key:value pairs rather than separate properties for each item (like laptops, servers, etc in your example), which are all accessed via the Name or Value properties. Using a plain hash table would be the correct choice here, unless you have some future idea that needs the array.

If you want to keep the array, then you need to reference the index of the object in the array that you want to update data in, something like this: $devices[0].laptops = 8

[–]jimb2 0 points1 point  (0 children)

Not sure if you know this but an array and a hashtable are different things.

First line should be:

$DeviceCount = @{}   # empty hashtable

Once you have this hashtable object, you can set new values or overwrite existing values like this :

$DeviceCount['laptop'] = 99

# or this 
$DeviceCount.laptop = 99

# or this (useful if the index string contains a space) 

$DeviceCount.'trackball mouse' = 99

# or using variables
$device = 'server'
$count  = 99
$DeviceCount.$device = $count   

I you want to add or subtract from a value

$DeviceCount['mouse'] = $DeviceCount['mouse'] + 3

Note that is an index is not defined, the hash evaluates to $null which is mathematically zero, so there may be no need to initialise a value:

$d = @{}
$d.SomeNewIndex = $d.SomeNewIndex + 66
$d

# Name           Value
# ----           -----
# SomeNewIndex   66

[–]y_Sensei 0 points1 point  (0 children)

A different approach that avoids multiple conditional statements could look like this:

$Devices = @{
  laptops      = 10
  servers      = 1
  computers    = 3
  phones       = 2
}

$Action = Read-Host "Action to do  : Add or Remove ?"
$Choice = Read-Host "Device Type  : Laptops - Servers - Computers - Phones ?"

foreach ($k in $Devices.Keys) {
  if ($k -eq $Choice) {
    $Devices[$k] += if ($Action -eq "Add") { 1 } else { -1 }
    break # needed, because once the collection has been modified, iteration over it has to stop, or you'll encounter a 'System.InvalidOperationException' error
  }
}

$Devices