Better way for many components?

I have coded a system where each 'Player' class requires 5 components (could be more in the future, up to 10, 20 maybe even 30, once we add more and more features). What I want some advice on is a better way of creating these components, I am not saying that my current way is bad, I am just looking for anyone who could throw a fresh idea my way, to give my application a much better feel component wise.

My current way:

_searchesComponent = new PlayerSearchComponent(_playerConnection);
_searchesComponent.InitializeComponent();

_ignoreComponent = new PlayerIgnoreComponent(_playerConnection);
_ignoreComponent.InitializeComponent();

_effectsComponent = new PlayerEffectComponent(_playerConnection);
_effectsComponent.InitializeComponent();

_itemComponent = new PlayerItemComponent(_playerConnection);
_itemComponent.InitializeComponent();

_logComponent = new PlayerLogComponent(_playerConnection);
_logComponent.InitializeComponent();

Here is the base layout of one of my component classes:

``
namespace MyApp.Players.Players.Effects
{
using System;
using System.Collections.Generic;
using Components;
using NLog;

internal sealed class PlayerIgnoreComponent : IDisposable, IComponent
{
private static readonly ILogger Logger = LogManager.GetCurrentClassLogger();

private readonly PlayerConnection _playerConnection;
private List _ignores;

public PlayerIgnoreComponent(PlayerConnection playerConnection)
{
_playerConnection = playerConnection;
_ignores = new List();
}

public void InitializeComponent()
{
using (var databaseConnection = Server.Database.NewDatabaseConnection)
{
var playerId = _playerConnection.SelectColumnInt("id");

databaseConnection.SetQuery("SELECT * FROM user_ignores WHERE user_id` = @userId");
databaseConnection.AppendParameter("userId", playerId);

Because the class is sealed, your IDisposable implementation is over-complicated. The whole entire point of the disposing flag is to make sure unmanaged resources get cleaned up correctly even if a class inherits this type and overrides Dispose... but your method isn't virtual, so it's only ever called/callable with Dispose(true).

Scrap that, keep it simple:

public void Dispose()
{
    _ignores.Clear();
    _ignores = null;       
}

Actually, if you're using that type correctly, then you gain nothing from explicitly clearing the internal list and nulling it up - the instance is [in theory] about to get collected anyway; IDisposable is making your class much more complex than it needs to be.

Typically you'd make a class implement IDisposable when it's holding on at instance level to a reference that implements IDisposable itself, which does not appear to be the case here (unless PlayerConnection has implications that aren't in your post) - disposing a List is pure overkill.

Moreover, calling Dispose twice should have no ill effect on the calling code, but in your case it throws a rather surprising NullReferenceException, and then you could very well call InitializeComponent again... without getting an expected ObjectDisposedException (you get another NullReferenceException instead).

The IComponent interface seems uncalled for, given how the callers seem to only be calling the concrete types' implementations of the InitializeComponent method.

The implementation is tightly coupled with some static/global-scope database-interacting service, which makes it untestable.

If all "components" load things from a database, it seems you need to look into the repository pattern, and possibly the unit of work as well. Consider using dependency injection to decouple your implementation from its dependencies, and avoid static / global-scope objects in OOP as a general rule of thumb.

StackExchange Code Review Q#158301, answer score: 3