Visual Studio Code – Is a blank try and catch IndexOutOfBoundsException valid at the end of a while loop of if statements in C#?

ID = convertStringtoID(comboBoxSlot1ID.Text);

This is a custom method. Rather than wrap it in a try/catch, make it return the default 0 value. While I’m here, that comboBox should also be given as an argument to the getImage() method (and the potriat/icon pair returned as a result, maybe in a tuple).

I understand you might not have that option, and assuming you have a legitimate need to use it instead of a simple int.Parse() or int.TryParse(), or if it’s used in other places and expected to throw. In that case, I then look at the catch value and see this:

ID = 0

And then notice this:

while (ID > 0 && ID < 906)

Put them together, and we can just exist the method right away.

try
{
    ID = convertStringtoID(comboBoxSlot1ID.Text);
}
catch ()
{
     return;
}

Looking more at this loop:

while (ID > 0 && ID < 906)

Nothing inside that loop ever changes ID, so the loop would run forever. You want if() instead of while().

I also see the conditional expressions used to decide how many 0s you need to prefix the ID value. You can replace all that like this:

pictureBoxSlot1Portrait.Image = (Image)Properties.Resources.ResourceManager.GetObject($"{ID:000}");
pictureBoxSlot1Icon.Image = (Image)Properties.Resources.ResourceManager.GetObject($"dot{ID:000}");

Another item, for the question about the empty try:

 else
 {
     try { }
     catch (System.IndexOutOfRangeException)
     {
         ID = 0;
     }
}

The try has nothing in it, least of all anything that would every throw an exception, and therefore the catch will NEVER run. It seems like you just want this:

else
{
    ID = 0;
}

And I need caution here about avoiding using exceptions for program logic control flow in the first place. It’s extremely poor practice. Rather, exceptions should reserved for events that are genuinely exception or unexpected.

But really, I’d skip that else() completely, since we already know logically from the checks made when entering the block that the value is in the expected range.

Finally, I saw this comment:

slot has a valid range of 1 to 3 … after the section of code I listed is the exact same code posted twice more, only used for slots 2 and 3 instead.

We can improve on that like this:

if (slot < 1 || slot > 3) return;
var portraits = new PictureBox[] {null, pictureBoxSlot1Portrait, pictureBoxSlot2Portrait, pictureBoxSlot3Portrait};
var icons = new PicutureBox[] {null, pictureBoxSlot1Icon, pictureBoxSlot2Icon, pictureBoxSlot3Icon};
var combos = new ComboBox[] {null, comboBoxSlot1ID, comboBoxSlot2ID, comboBoxSlot3ID};

And then use the new array variables when needed:

ID = convertStringtoID(combos[slot].Text);
// ...
portraits[slot].Image = (Image)Properties.Resources.ResourceManager.GetObject($"{ID:000}");
icons[slot].Image = (Image)Properties.Resources.ResourceManager.GetObject($"dot{ID:000}");

Even better if the arrays are defined for this at the class level, but I’ll leave that change for another time.

Put it all together, and the method reduces to this:

void getImage(int slot)
{
   if (slot < 1 || slot > 3) return;

   var portraits = new PictureBox[] {null, pictureBoxSlot1Portrait, pictureBoxSlot2Portrait, pictureBoxSlot3Portrait};
   var icons = new PicutureBox[] {null, pictureBoxSlot1Icon, pictureBoxSlot2Icon, pictureBoxSlot3Icon};
   var combos = new ComboBox[] {null, comboBoxSlot1ID, comboBoxSlot2ID, comboBoxSlot3ID};

   short ID = 0;
   try {
      ID = convertStringtoID(ID = convertStringtoID(combos[slot].Text););
   } catch() { return; }

   if (ID <= 0 || ID >= 906) return;

   portraits[slot].Image = (Image)Properties.Resources.ResourceManager.GetObject($"{ID:000}");
   icons[slot].Image = (Image)Properties.Resources.ResourceManager.GetObject($"dot{ID:000}");     
}

This now works with only about 1/6 the code of the original. That it will also be faster is just a bonus.

There are further improvements that can be made as well, to better decouple the UI from the back end and simplify the code, but this is enough for today. But it’s worth repeating here that at no point is an exception ever thrown or even tested. No try/catch is needed.