Design of class interaction

I'm contemplating the design of classes, playing with a simple hypothetical design, and wondering if there's some general principle of good design this violates, and if there's a better way.

Say I have a class of Messages. Each message is a string. The Messages class holds a list of all the message strings.

I want to send these messages to their destination. Say, write them to a file. So I have a MessageSender class which does the job.

Class Diagram

using System;
using System.Collections.Generic;
using System.IO;

namespace TestMessages
{
    class Program
    {
        static void Main(string[] args)
        {
            var messages = new Messages();
            var messageSender = new MessageSender();
            messageSender.SendMessages();
        }
    }

    public class Messages
    {
        public static List<string> myMessages;
        public Messages()
        {
            myMessages = new List<String>(new string[] { "one", "two", "three" });
        }
    }

    class MessageSender
    {
        public void SendMessages()
        {
            using (StreamWriter writer = new StreamWriter("myFile.txt"))
            {
                foreach (string s in Messages.myMessages)
                {
                    writer.WriteLine(s);
                }
            }
        }
    }

}

Open in new window

This all works. But I see problems.

First there's the obvious static List in the Messages class, which makes it global and easily accessible, and abusable.

And the Messages class is just data with no behavior.

Second, I see MessageSender reaching in to class Messages and extracting the data. This is the one I'm not sure if it violates encapsulation, or some other good design principle.

I can make the MyMessages list not static:

Class Diagram
using System;
using System.Collections.Generic;
using System.IO;

namespace TestMessages
{
    class Program
    {
        static void Main(string[] args)
        {
            var messages = new Messages();
            var messageSender = new MessageSender(messages);
            messageSender.SendMessages();
        }
    }

    public class Messages
    {
        public List<string> myMessages;
        public Messages()
        {
            myMessages = new List<String>(new string[] { "one", "two", "three" });
        }
    }

    class MessageSender
    {
        private Messages messages;

        public MessageSender(Messages messages)
        {
            this.messages = messages;
        }

        public void SendMessages()
        {
            using (StreamWriter writer = new StreamWriter("myFile.txt"))
            {
                foreach (string s in messages.myMessages)
                {
                    writer.WriteLine(s);
                }
            }
        }
    }

}

Open in new window

I'm feeling like maybe SendMessages() should take some messages as a parameter:

Class Diagram

using System;
using System.Collections.Generic;
using System.IO;

namespace TestMessages
{
    class Program
    {
        static void Main(string[] args)
        {
            var messages = new Messages();
            var messageSender = new MessageSender();
            messageSender.SendMessages(messages);
        }
    }

    public class Messages
    {
        public List<string> myMessages;
        public Messages()
        {
            myMessages = new List<String>(new string[] { "one", "two", "three" });
        }
    }

    class MessageSender
    {
        public void SendMessages(Messages messages)
        {
            using (StreamWriter writer = new StreamWriter("myFile.txt"))
            {
                foreach (string s in messages.myMessages)
                {
                    writer.WriteLine(s);
                }
            }
        }
    }

}

Open in new window

Or I could move SendMessages() into the Messages class:

Class Diagram
using System;
using System.Collections.Generic;
using System.IO;

namespace TestMessages
{
    class Program
    {
        static void Main(string[] args)
        {
            var messages = new Messages();
            messages.SendMessages();
        }
    }

    public class Messages
    {
        public List<string> myMessages;
        public Messages()
        {
            myMessages = new List<String>(new string[] { "one", "two", "three" });
        }

        public void SendMessages()
        {
            using (StreamWriter writer = new StreamWriter("myFile.txt"))
            {
                foreach (string s in myMessages)
                {
                    writer.WriteLine(s);
                }
            }
        }
    }

}

Open in new window

It kind of looks better; however, now i have a reference to an output file in my Messages class. I don't think Messages include an output file. Also, do messages really know how to send themselves to a file?

I can move the file reference to a parameter. But I still have my Messages class doing I/O to a file. Should a Messages class know about files and I/O?

Class Diagram
using System;
using System.Collections.Generic;
using System.IO;

namespace TestMessages
{
    class Program
    {
        static void Main(string[] args)
        {
            var messages = new Messages();
            messages.SendMessages("myFile.txt");
        }
    }

    public class Messages
    {
        public List<string> myMessages;
        public Messages()
        {
            myMessages = new List<String>(new string[] { "one", "two", "three" });
        }

        public void SendMessages(string filename)
        {
            using (StreamWriter writer = new StreamWriter(filename))
            {
                foreach (string s in myMessages)
                {
                    writer.WriteLine(s);
                }
            }
        }
    }

}

Open in new window


So I guess my questions are:
1. Should Messages know how to send themselves?
2. Is it improper for another class to reach into the Messages class, grab it's data, and do something with it's data? What principle does this violate?
deleydAsked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Nitin SontakkeDeveloperCommented:
Despite being a developer for several years now, I haven't really looked deeply (and academically) into Design patterns formally as such. I do admit that I am guilty of that sin.

Having said that, I do try my best not to increase any dependency of one class over another. What they formally love to call 'separation of concern' which I guess just other side of the same coin, 'encapsulation'.

In this particular scenario, if I have to add my ignorant 2 cents, I would probably never have a Messages class. I will just have a Message class. In your demo only example, it is just a string, however, as you know it all starts getting complicated in real life.

The message class itself can have a private List<Message> declared and several helper functions to deal with the list, such as AddMessage, GetMessage, RemoveMessage and GetMessages. Of course, if you wish there is no need to complicate it this far, as just leave the List property public and have it initiased in a static constructor so as to avoid any Null exceptions.

Then you have SendMessage class, which would probably have several overloaded SendMessages methods which would take List of messages and expected output mode, as in:

SendMessages(List<Message> messages, String outputFile);
SendMessages(List<Message> messages, MailConfiguration mailConfiguration); // smtp, I mean

In fact, you can have just Send method to Message class as well. A separate class makes sense only in a scenario you have something like IMessage interface and then have a separate class which would take List<IMessage> or IMessage type as a parameter. Otherwise, it doesn't make much sense, I suppose.
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Programming

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.