Assertions, Assertions, Assertions - How much is too much?

[cafeenman]

PS. What I'm specifically asking about is when the same thing is being asserted multiple times as it gets passed from procedure to procedure.

This is all in place and working very well, thank you.

But as I review the code for the brazillionth time, I'm wondering about when an Assertion should be made.

What is going on is Items. Specifically passing them around.

I can make as many Item Pools (collection class) as I want.

I have these Items classes:

AllItems - Always holds all Items. This one is just an easy place to look up any item. It does nothing in the app except be a bag of everything.
WildItems - these are the items you find in the wild when you dig a hole or explore a cave or whatever
EquippedItems
BackpackItems
VendorItems

AllItems withstanding, no item can be in two places at the same time so I have to be really careful about ensuring items are where I think they are.

So lots of assertions.

I can see three arguments about them.

1) Assert as early as possible to skip out of code quickly if the assertion fails.

2) Don't assert until it actually matters if the thing is true even if that means running a bunch of code I could have skipped if the assertion fails.

3) Always assert everything even if that's less efficient. This is mostly the route I've taken.

Also, Items can be randomly created in the game. These Items are always flagged as Destroy on Unequip.

Thus you can buy it, store it, sell it, lose it, equip it. Once equipped you can use it forever until you take it off and then it's gone forever.

Public Function TryBuyItem(Optional ByRef ItemSlotID As ITEM_SLOT) As Long
Dim m_Item As cItem

Set m_Item = RandomItemSlotItem(ItemSlotID, VendorItems)

BuyItemDialog ItemSlotID

If CanBuyItem(m_Item) <> 1 Then Exit Function

If ItemIsGift(m_Item) <> 1 Then

  If BuyItem(m_Item) <> 1 Then Exit Function

End If

EquipOrStoreNewItem m_Item, VendorItems

TryBuyItem = 1

End Function

Private Property Get RandomItemSlotItem(ByRef ItemSlotID As ITEM_SLOT, ByRef Items As cItems) As cItem
Dim m_Item As cItem
Dim colTemp As Collection

If Not ValidObject(Items) Then Exit Property

If Items.Count = 0 Then Exit Property

Set colTemp = New Collection

For Each m_Item In Items

  With m_Item

    If .ItemSlot = ItemSlotID Then colTemp.Add m_Item

  End With

Next m_Item

If colTemp.Count = 0 Then Exit Property

Set RandomItemSlotItem = colTemp(RollDie(colTemp.Count))

End Property

Private Sub EquipOrStoreNewItem(ByRef Item As cItem, ByRef FromItems As cItems)
Dim m_EquippedItem As cItem

If RollDie(d2) = 1 Or Not CanEquipItem(Item) Then  ' Store Item in the Backpack.

  TransferItem Item, FromItems, BackpackItems

Else ' Equip Item.

  Set m_EquippedItem = EquippedItem(Item.ItemSlot)

  TransferItem m_EquippedItem, EquippedItems, BackpackItems

  TransferItem Item, FromItems, EquippedItems

End If

Public Function TransferItem(ByRef Item As cItem, ByRef FromItems As cItems, ByRef ToItems As cItems) As Long

If Not ValidObject(Item) Then Exit Function
If Not ValidObject(FromItems) Then Exit Function
If Not ValidObject(ToItems) Then Exit Function

If ToItems Is EquippedItems Then

  If Not CanEquipItem(Item) Then Exit Function

End If

FromItems.Remove Item

If (FromItems Is EquippedItems) And (Item.DestroyOnUnequip = vbChecked) Then

  ' Do nothing. All the Item's stats are destroyed with the Item.
  ' It is safe to keep it in AllItems as Items are never drawn from it.
  ' The downside is that AllItems will end up with tens of thousands of Items in it that aren't doing anything.

  ' Or... go ahead and destroy it. Screw stats.

  'AllItems.Remove Item

  'Set Item = Nothing

Else

  ToItems.Add Item

End If

TransferItem = 1

End Function

Private Function CanBuyItem(ByRef Item As cItem) As Long
Dim nLevelDifference As Long
Dim nRemainingBalance As Long

If VendorItems.Count = 0 Then Exit Function

If Not ValidObject(Item) Then

  ItemNotAvailable

  Exit Function

End If

nRemainingBalance = (McGuffin.Count - Item.Value - MIN_MCGUFFINS_TO_ACTIVATE_MULTIPLIERS)

If nRemainingBalance < 0 Then

  AddMessage AddAsterisks("You can't afford a " & Item.Name & " (" & -Item.Value & ")."), vbTab, 0, idx_PlayerMessage_Primary

  Exit Function

End If

nLevelDifference = Item.LevelRequirement - Player.Level

If nLevelDifference > 0 Then

  If RollFor("Buy Deck-Requirement-Not-Met Item", nLevelDifference, idx_PlayerMessage_Notification) <> nLevelDifference Then

    AddMessage AddAsterisks("You Have Not Reached the Deck " & Item.LevelRequirement & " Requirement to Equip a " & Item.Name & "."), vbTab, 0, idx_PlayerMessage_Primary

    Exit Function

  End If

End If

CanBuyItem = 1

End Function

[cafeenman]

Man do I need to learn to comment my code.

I just spent too many minutes trying to remember what this does:

nLevelDifference = Item.LevelRequirement - Player.Level

If nLevelDifference > 0 Then

  If RollFor("Buy Deck-Requirement-Not-Met Item", nLevelDifference, idx_PlayerMessage_Notification) <> nLevelDifference Then

    AddMessage AddAsterisks("You Have Not Reached the Deck " & Item.LevelRequirement & " Requirement to Equip a " & Item.Name & "."), vbTab, 0, idx_PlayerMessage_Primary

    Exit Function

  End If

End If

You *can* purchase any item if you have the funds even if the level requirement is much higher than your current level.

The number of Die Sides is the difference in the level requirement from your level.

Thus it is less probable to roll a high-level item. For example, if the item is 20 levels higher then you have to roll a 20 on a D20 to get it.

You will never fail a roll that is one level higher than your level or less because rolling a D1 will always return 1.

That's what it does.

I should write this in the code now so I don't have to read the code next time.

That is all.

[qvb6]

For a while I thought you were talking about Debug.Assert, but it seems that you are not.

Validation checks are better put inside functions(as you may have discovered, like IsValid()) because you can forget some of the rules. For example, if you always need to check for a flag and a counter each time you access an array element, you will forget that rule a year later, and introduce bugs. A function checks all the rules. Yes, it may slow things down but a reliable code is better than fast buggy code.