80O / PlusEMU

37 stars 28 forks source link

Refactoring examples #195

Open 80O opened 3 months ago

80O commented 3 months ago

Couple refactoring examples I would like to share for new people to get started. Its important to note that your changes & PRs stay small so they're easy to merge and resolve conflicts. This examples will give you a general idea on where and how you can get started.

I'll go over each line by line and put comments on what is best to do. The links are permalinks so future changes have not been reflected yet and it might have changed in the meantime.

Example 1: GetCatalogPageEvent


public class GetCatalogPageEvent : IPacketEvent
{
    private readonly ICatalogManager _catalogManager;

    public GetCatalogPageEvent(ICatalogManager catalogManager)
    {
        _catalogManager = catalogManager;
    }

    public Task Parse(GameClient session, IIncomingPacket packet)
    {
        var pageId = packet.ReadInt();
        packet.ReadInt(); // Skipping part of the packet should be assigned to a variable. This helps future development. If unknown, use var unknown1, var unknown2 etc.
        var cataMode = packet.ReadString(); // Naming is important. As we're already in a catalog packet, I think its sufficient to just call this mode instead of cataMode.

        if (!_catalogManager.TryGetPage(pageId, out var page))
            return Task.CompletedTask;

// It might be better to move this logic entirely to the Page and call it something like HasAccess(Habbo habbo)
        if (!page.Enabled || !page.Visible || page.MinimumRank > session.GetHabbo().Rank || page.MinimumVip > session.GetHabbo().VipRank && session.GetHabbo().Rank == 1) // Personally not really a fan of chaining all if conditions. I'd rather see them grouped. Example page.Enabled and page.Visible can be checked together as they only apply to the page.
            return Task.CompletedTask;

        session.Send(new CatalogPageComposer(page, cataMode));
        return Task.CompletedTask;
    }
}```

TBA