Object oriented style - where to define properties? VBA

Question

I'm quite green on object oriented programming and trying to get my style right.

Very often I have an object of which one the properties is a dictionary containing other objects. Call it class 'Team' that contains a class 'Player'.

Now let's say every time a Player is added to a Team I would like to have the average age of the team updated.

My favorite solution:

In Sub Main I should say just

 Team.add(Player)

Then in Team the method add is:

 Public Sub Add(Player As CPlayer):
      pPlayers.Add Player.Name, Player
      Me.UpDateAvgAge(Player.Age)       
 End Sub

Now I can imagine at least one alternative way to do that which is:

In Main:

Team.add(Player)
Team.UpDateAvgAge(Player.Age)

And the add method should off course not have the Me.UpDateAvgAge(Player.Age) line.

No need to say this is just simplest example. In real life there are a number of properties being 'updated' every time I 'add' something.

Is there a consensus among programmers on how to do this add/update? Any guidelines?

tks in advance!

In case of average, what I would do, is not to have special field with name `averageAge` in `Team` class, but add simple calculated `Get AvarageAge` property (without background field) that would calculate average age each time you call it. As I know it's general practise for such properties (at least in another OOP languages) — Dmitry Pavliv, Apr 01 '14 at 08:30
@simoco. Point well taken, but really avg is just and example. — , Apr 01 '14 at 08:54
in that case your first option was better than second. You shouldn't allow the user to intervene in the logic of the class - all of this calculations should be hidden inside the class implementation — Dmitry Pavliv, Apr 01 '14 at 09:04
further to simoco's first comment. Almost any calculable property should be returned on fly. It's how it's always designed, you don't store any calculable properties in variables. In any calculable case based on a dictionary, you iterate the dictionary and add up all the `Age`s and then divide that by the number of items in the dictionary. — , Apr 01 '14 at 09:12
@mehow. Okay, that's where I lack the big picture. Why is that the case? Doesn't the general rule that memory is cheaper then performance suggest otherwise? — , Apr 01 '14 at 10:27
One reason is simply that if somewhere else in your code you were to `if birthday then player.Age = player.Age + 1` then calculating the average on the fly by iterating your collection would produce the correct result with no need to remember to update the backing field. — Alex K., Apr 01 '14 at 11:23
*memory is cheaper than performance* - This is something you need only consider when the performance hit of calculating something in realtime has a demonstrable impact on the process. — Alex K., Apr 01 '14 at 11:24
@VBOG you actually execute more operations cause with each player you add to collection you need to calculate the average age (1 + 1 *n), but you don't need that average age each time you add a player (b/c you're adding a player). You only need to **know the average when it's requested.** You only need to once iterate the collection to retrieve it and by calculating it each time a player is added you execute repeatedly the same thing. One of the main principles of OOP is the [**DRY Principle**](http://en.wikipedia.org/wiki/Don't_repeat_yourself). The OOP solution 3 comments above this one. — , Apr 01 '14 at 11:27
Consider adding 50K players, each time player is added you are updating average. At the end of the programs cycle you have executed 50K calls to add a Player to collection plus 50K times you calculated the average Age. It's not an every day scenario that you would need to do that for any reason ( trace stock market and MVA etc *maybe yes* ) but not in a simpler case. If you only added 50K players and needed to know the average Age **once** at the end than obviously you would only make 1 call to get the average. What do you like better now 50K+50K calls or 50K+1 call? — , Apr 01 '14 at 11:33
@mehow. Clear. Also clear that my simplified example with average was not the best one. But your point is I should make the math. But maybe for me the main point is that most people reading an OOP code will expect it to happen in the 'get' method. So I should think three times before putting it someplace else. — , Apr 01 '14 at 12:32
@VBOG in your example it doesn't make sense to not do what simoco and mehow are suggesting. — enderland, Apr 01 '14 at 15:17
someone should make an answer out of the comments above. im re-writing my entire code based on it and, with minimum compromise, the code is just getting so so much better — , Apr 04 '14 at 06:25

score 0 · Accepted Answer · answered Apr 07 '14 at 15:34

I could imagine something like this. Class CTeam would precalculate sum of ages of team members right at the moment when new player is added to the collection. And then just use this number instead of having to loop through all the items and calculate the number. But you'll have to update the value in case member is removed from collection etc. You have to decide if this work is suitable or if you just calculate all the values at the moment when it is needed (means no caching).

You could add and event to the CTeam class as well and so let the subscribers know that new team member was added to the collection. Like in this example UserForm1 is subscriber and it gets informed when publischer which is class CTeam creates new player. HTH

' CTeam Class:

Public Event PlayerWasAdded(player As CPlayer)

Private m_players As VBA.Collection
Private m_sumTeamAge As Single

Private Sub Class_Initialize()
    Set m_players = New VBA.Collection
End Sub

Public Sub Add(player As CPlayer)
    m_players.Add player, player.Name
    m_sumTeamAge = m_sumTeamAge + player.Age
    RaiseEvent PlayerWasAdded(player)
End Sub

Public Property Get AverageAge() As Single
    If m_players.Count > 0 Then
        AverageAge = m_sumTeamAge / m_players.Count
    Else
        AverageAge = 0
    End If
End Property

' UserForm1 Class:

Private WithEvents m_team As CTeam

Private Sub UserForm_Initialize()
    Dim i As Integer

    Set m_team = New CTeam

    For i = 1 To 5
        m_team.Add CreateNewPlayer(i)
    Next
End Sub

Private Sub m_team_PlayerWasAdded(player As CPlayer)
    MsgBox "New player " & player.Name & " was added. Do something ...", vbInformation
End Sub

Private Sub ShowAverageAge_Click()
    MsgBox "Average age of team member is: " & m_team.AverageAge, vbInformation
End Sub

Private Sub AddNewPlayer_Click()
    m_team.Add CreateNewPlayer(...)
End Sub

Private Function CreateNewPlayer(platerIndex As Integer) As CPlayer
    Dim upperbound As Integer: upperbound = 35
    Dim lowerbound As Integer: lowerbound = 18
    Dim player As CPlayer

    Set player = New CPlayer
    player.Name = "Player_" & platerIndex
    player.Age = Int((upperbound - lowerbound + 1) * Rnd + lowerbound)
    Set CreateNewPlayer = player
End Function

In my case the key was moving the stuff like average to the `Get` method. I did lose a bit in performance (helps that I'm using dictionaries), but gained a ton in code clarity and usability. — , Apr 08 '14 at 11:01

Object oriented style - where to define properties? VBA

1 Answers1