C# static class holding list of member class instances

I was wondering if such a design was a bad idea in C#. Here I have a static class EventLog which holds a list of instances of Event class. This is a simple event log of actions players can do in a game. What can I be doing in a better way?

public class EventLog
{
    private const int maxEventsInLog = 50;
    private static List events = new List();
    private static string[] eventDescriptions = new string[8]
    {
        " walked",
        " waited",
        " inquired",
        " allured ",
        " persuaded ",
        " bribed ",
        " hired ",
        " attacked ",
    };

public static void AddEvent(GameObject _fromUnit, GameObject _toUnit, Utils.ActionType _actionType)
{
    events.Add(new Event(_fromUnit, _toUnit, _actionType));

    if (events.Count > maxEventsInLog)
    {
        while (events.Count > maxEventsInLog)
        {
            events.RemoveAt(0);
        }
    }
}

public static int NumEventsInLog()
{
    return events.Count;
}

public static string GetEventDescription(int eventIndex)
{
    if (eventIndex  events.Count - 1)
        return null;

    return events[eventIndex].description;
}

private class Event
{
    private GameObject fromUnit;
    private GameObject toUnit;
    private Utils.ActionType actionType;
    public string description;

    public Event(GameObject _fromUnit, GameObject _toUnit, Utils.ActionType _actionType)
    {
        fromUnit = _fromUnit;
        toUnit = _toUnit;
        actionType = _actionType;

        description = eventDescriptions[(int)_actionType];
        description = description.Replace("", _fromUnit.GetComponent().firstName + " " + _fromUnit.GetComponent().lastName);
        description = description.Replace("", _toUnit.GetComponent().firstName + " " + _toUnit.GetComponent().lastName);
    }
}

I think your tokens ` and should be public static readonly string fields, instead of being hard-coded the way they are. This turns eventDescriptions into something like this:

private static string[] eventDescriptions = new string[8]
{
    FromUnitToken + " walked",
    FromUnitToken + " waited",
    FromUnitToken + " inquired",
    FromUnitToken + " allured " + ToUnitToken,
    FromUnitToken + " persuaded " + ToUnitToken,
    FromUnitToken + " bribed " + ToUnitToken,
    FromUnitToken + " hired " + ToUnitToken,
    FromUnitToken + " attacked " + ToUnitToken,
};

And then the Event constructor can do something like this:

description = description.Replace(EventLog.FromUnitToken, _fromUnit.GetComponent().firstName + " " + _fromUnit.GetComponent().lastName);
    description = description.Replace(EventLog.ToUnitToken, _toUnit.GetComponent().firstName + " " + _toUnit.GetComponent().lastName);

The description field is problematic though:

public string description;

Instance fields shouldn't be public - that's breaking encapsulation, anyone that can use an Event instance can do whatever they want with that value. Better encapsulate it:

private readonly string description;
public string Description { get { return description; } }

Fields initialized from constructor should be marked readonly if that's the intent - it makes the intent clearer.

In AddEvents, you're adding events, and then removing whatever exceeds your max capacity:

events.Add(new Event(_fromUnit, _toUnit, _actionType));

if (events.Count > maxEventsInLog)
{
    while (events.Count > maxEventsInLog)
    {
        events.RemoveAt(0);
    }
}

Looks like you want some First In, First Out data structure - a List just isn't practical for that - consider using a Queue` instead.

StackExchange Code Review Q#78035, answer score: 5