Best practices for object modelling

[dennison]

I am trying to write code for a class that has a couple of attributes that will can only be known upon creation. Because I think accessor functions (e.g. getSomething()) are outdate, I declared my attributes as "public readonly" in order to protect the attributes from getting modified yet still accessible by the parent class. I am not convinced that this is the best practice for this. Implementing properties seem like a bit of a waste of time to me. (refer to this article to see what I mean)

Another issue of mine is how trapping exceptions during class creation. I know having a blank catch statement is bad, but I do not want my class to thrown an exception if something went wrong. I have no problems with this because the default values of the properties will already tell me that the class was not properly created, but again I do not think this is the best practice.

To explain further, please see my code below:

    class MessageObject
    {
        public readonly int Length = 0;
        public readonly int Sequence = 0;
        public readonly bool Duplicate = false;
        public readonly string Data = "";
        public readonly string Message = "";

        public MessageObject(string message)
        {
            try
            {
                Message = message;
                Length = int.Parse(message.Substring(0, 3));
                Sequence = int.Parse(message.Substring(3, 6));
                Duplicate = message.Substring(9,1).ToLower() == "y" ? true : false;
                Data = message.Substring(10, Length);
            }
            catch
            {
                // Do something
            }
        }

Help!

[dennison]

I just realized that if I do a try ... catch some properties may get set while some won't, so I decided to eliminate the try ... catch block. Am I stuck with having to catch possible exceptions in the parent class?

[wimpos]

Hi

I can agree with your decision to not use properties, though I always do use properties to expose private members to the outside world. But that 's a personal decision.

About your code. To save me a lot of blabla I rewrote your class in a way I would write it. Ofcourse it is not THE solution, but in my opinion it does what you want and it is "cleaner"

class MessageObject
	{
		private int length = 0;
		public int Length
		{
			get { return length; }
		}

		private int sequence = 0;
		public int Sequence
		{
			get { return sequence; }
		}

		private bool duplicate = false;
		public bool Duplicate
		{
			get { return duplicate; }
		}

		private string data = string.Empty;
		public string Data
		{
			get { return data; }
		}

		private string message = string.Empty;
		public string Message
		{
			get { return message; }
		}

		public MessageObject(string message)
		{
			this.message = message;

			// I never write logic in my constructor
			// call a method
			ParseMessage();
		}
		
		// this method parses the message injected in the constructor
		private void ParseMessage()
		{
			// For every parsing line I first check if the message string
			// is actually long enough to get the substring
		   // you could also do this at once by using 10 as the min value
			if (message.Length >= 3)
			{
				// the try parse method, does the same as parse,
				// but it returns false instead of throwing an exception
				// the integer to store the parsed value in, is parameter with 				//the OUT keyword
				int.TryParse(message.Substring(0, 3), out length);
			}
			if (message.Length >= 6)
			{
				int.TryParse(message.Substring(3, 6), out sequence);
			}
			if (message.Length >= 10)
				duplicate = message.Substring(9, 1).ToLower() == "y" ? true : false;

			if (message.Length >= 10 + Length)
				data = message.Substring(10, Length);
		}
	}

extra tips:

• for every hardcoded int (message length, and ints for substring) make it a const at the top of your class, or add it as a setting, so these values can be edited without changing code.
• instead of calling the ParseMessage from the constructor, you could set it public and give it a return value (true for successful parsing, false otherwise)

Hope this helps

Kind regards
Wim

[dennison]

Hi Wim. I realy appreciate what you did. Am I correct in assuming that your reason for separating logic from the constructor is in to make it easier to add alternate constructors?

Thank you.

[r035198x]

I find all this alarming. The decision for whether or not to use a property must come from the problem specification. Is length a property of a Message object? So it's not really about what is easier to write or what looks clean but rather, what is the situation that we are trying to model.

And everything you've heard about blank catches is correct.

[dennison]

I find all this alarming. The decision for whether or not to use a property must come from the problem specification. Is length a property of a Message object? So it's not really about what is easier to write or what looks clean but rather, what is the situation that we are trying to model.

And everything you've heard about blank catches is correct.

What is so alarming about that? Variables pretty much serve the same purpose as properties. When I wrote JAVA code 10 years ago I had to make use of private variables and modify them through set and get functions.

[r035198x]

What is so alarming about that? Variables pretty much serve the same purpose as properties. When I wrote JAVA code 10 years ago I had to make use of private variables and modify them through set and get functions.

What's alarming is the criteria the OP is using for deciding when to use properties and when not to use.
I use Java myself sometimes but I don't make my properites private and provide getters and setters for them as a rule. The problem I'm solving decides whether a property should be "settable" through a public method or not.