CODEWITHSUNDEEP
c#refactoring
user1320651
user1320651

Reputation: 836

How would I refactor this code to reduce CC

Im just curious to know as this code has a code complexity of 16, if I want to hit the perfect 9/10 on CC, what would be the best approach? I know its not going to kill me but I am keen to learn if other people would write this code differently

class SentMessages
{
    public SentMessages() { }
    public SentMessages(int id, string userName, string message, string messageType, DateTime createdAt)
    {
        this.Id = id;
        this.UserName = userName;
        this.Message = message;
        this.CreatedAt = createdAt;
        this.MessageType = messageType;

    }
    public string UserName { get; set; }
    public int Id { get; set; }
    public string Message { get; set; }
    public string MessageType { get; set; }
    public DateTime CreatedAt { get; set; }

    public List<SentMessages> GetMessages()
    {
        if (AllMessages.Count == 0) AllMessages = SentMessages.InitializeMessages();
         return AllMessages;
    }
    public List<SentMessages> AllMessages = new List<SentMessages>();
    static private List<SentMessages> InitializeMessages()
    {
        List<SentMessages> messages = new List<SentMessages>();
        return messages;
    }
    public void ClearMessages()
    {
        AllMessages.Clear();
    }
}

I've gained the concept for this code from http://objectlistview.sourceforge.net/cs and a stackoverlfow answer

Upvotes: 0

Views: 99

Answers (2)

Squirrel5853
Squirrel5853

Reputation: 2406

public class SentMessage : Message, IMessage
{
    public SentMessage(int id, string userName, string message, string messageType, DateTime createdAt)
    {
        Id = id;
        UserName = userName;
        Message = message;
        CreatedAt = createdAt;
        MessageType = messageType;
    }

    public string UserName { get; private set; }
    public int Id { get; private set; }
    public string Message { get; private set; }
    public string MessageType { get; private set; }
    public DateTime CreatedAt { get; private set; }
}

public class MessageCollection<T> where T : IMessage
{
    private List<T> _messages;

    public MessageCollection()
    {
       _messages = new List<T>();
    }

    public IEnumerable<T> GetMessages()
    {
        return _messages;
    }

    public void AddMessage(T message)
    {
        _messages.Add(message);
    }

    public void ClearMessages()
    {
        _messages.Clear();
    }
}

Upvotes: 1

Myrtle
Myrtle

Reputation: 5831

I think you should consider the principle

Seperation of Concern

Just some thoughts:

  • Create a factory class to instantiate a new collection.
  • Create a Message' class as a dataholder to keep the Id, Username properties
  • Create a custom SentMessages class that implements something like IList<SentMessage>

Oh, and GetMessages makes no sense. It is invoked when the list is empty. However, it will create a new empty list.

Upvotes: 2

Related Questions