Leaderboard Names

[Draekdude]

Leaderboard Names

OK, when we started to work on the LeaderboardEntityDisplay, I immediately thought we should be sending the LeaderboardEntityState struct rather than each field independently since we already had that created.

So I did:

    public void Initialize(LeaderboardEntityState leaderboard)
    {
        ClientId = leaderboard.ClientId;
        _playerName = leaderboard.PlayerName.ToString();
        UpdateCoins(leaderboard.Coins);
    }

And then in the LeaderBoard method HandleLeaderboardEntitiesChanged:

                LeaderboardEntityDisplay leaderboardEntityDisplay = Instantiate(leaderboardEntityPrefab, leaderboardEntityHolder);

                leaderboardEntityDisplay.Initialize(new LeaderboardEntityState
                {
                    ClientId = changeEvent.Value.ClientId,
                    PlayerName = changeEvent.Value.PlayerName,
                    Coins = changeEvent.Value.Coins
                });
                entityDisplays.Add(leaderboardEntityDisplay);

I figured that even though LeaderboardEntityState has the network serlializer, it could still be used. Is this appropriate?

Thanks,

Julian Boardman

[Yitzchak_Cohen]

Hi there,
Your method works, and while it looks nicer, it actually slightly less ideal in terms of object oriented programing. Let me try and explain why.

When we are calling to a method in another class, we want to pass the data in using the simplest forms possible. This is because we don’t want to be dependent on the data structure for passing the parameters. Calling from one class to the other should not depend on a third class or struct, to make that call if you don’t have to.

For example, in the case you provided, we are initially just passing strings and ints, very basic data types. When you pass the LeaderboardEntityState you are now making the this function call dependent on the structure of that struct. Let’s say you were to change that struct and add or remove one of the fields from it. I would now have to update the HandleLeaderboardEntitiesChanged method to add or remove that extra field when I initialize the struct. Now if you called Initialize from a lot of different places, you would have to go update them. If you had stuck with the simple types, you could update that struct without having to update any of the calls to Initialize.

Perhaps this is a dependency you want (maybe to ensure the leaderboard display always gets the complete data set)? But in general it is better to have fewer dependencies, that way if you make a change it does not break your entire program.

Hope this makes sense! It’s not an intuitive part of object-oriented programming, so it might take a second to wrap your head around it. You can try playing with your code and see how changing the struct effects your function calls.

[Draekdude]

Hmmm. Interesting. I think for under 6 fields what you say is true and accurate. However, if there were 6+ fields I think it would be best to create a class called PlayerData and then put all of those items into that player data object and then pass it.

My laziness took over and I should have just created a new class, but the simple fact is that HandleLeaderboardEntitiesChanged is already tightly coupled to LeaderboardEntityState. Look at the signature:
private void HandleLeaderboardEntitiesChanged(NetworkListEvent<LeaderboardEntityState> changeEvent)

If you remove ClientId from LeaderboardEntityState you would immediately have to change HandleLeaderboardEntitiesChanged to stop it from being passed from the changeEvent. If you meant that I am now tightly coupling it to LeaderboardEntityDisplay, then I agree. But the point of that class is to display what is in the LeaderboardEntityState. We often pass objects between classes when we don’t need every single parameter in the class. But it’s much easier to see what’s going on rather than passing each parameter individually.

A very important part of developing is readability. Myself, I make sacrifices to make code more readable. If you are passing 6+ parameters instead of just throwing them into an object you are drastically reducing the readability of your code and exposing your methods for bugs due to them being less readable. Of course just passing 6+ parameters into a single method isn’t terrible, but what if that method calls another method with those same parameters? Now it’s getting confusing and complex.

Perhaps I was attempting to solve a problem before it’s a problem. But I was just thinking since it’s already tightly coupled, why not just use it. If it were a class instead of a struct, I would have made a base class called LeaderboardData and then just passed that and also had LeaderboardEntityState inherit from it.

Thanks for the discussion!

Julian

[Yitzchak_Cohen]

I totally agree this could go both ways here. In the context of our game it makes sense to just pass that state, as that is the reason we created all the classes in this context. I also agree that’s the best way to ensure we pass all the correct data to the leaderboardEntityDisplay without getting any errors.

However, I am not sure I agree on the point that if they are tightly coupled already, then it’s okay to pass the struct. Mainly, what if we had an entirely different class that needed to initialize a LeadboardEntityDisplay? That class might not be coupled to the LeaderBoardEntityState, but now it would have to be in order to Initialize the LeadboardEntityDisplay. We could create an overload method I suppose that just takes the parts and not the full struct. But we should be theoretically able to Initialize a LeadboardEntityDisplay from other classes.

I also think doubling down on coupling the HandleLeaderboardEntitiesChanged to the leaderboardEntityState isn’t the best practice. The stronger the coupling, the more difficult it is to change later. Though in this context, it is pretty easy to argue that the gains of organization outweigh the slightly stronger coupling.

I like your idea of creating a data class just to pass this specific data around. That would loosen the coupling a little while maintain the readability. In that case you would still want to weigh the dependency on the struct to the flexibility of not using one.