Using C# properties for simple operations, where should I draw the line?

Question

Should I be putting Linq statements inside of an objects properties? Is that a best practice or is that a no no? Also if that is ok, where do I draw the line with this? I assume db access is probably too far but is there a line, or just a gray area? Do you see any issues with my usage of Linq?

[XmlRoot("message")]
public class Message
{
    [XmlAttribute]
    public string type { get; set; }
    [XmlElement("segment")]
    public Segment[] segments { get;set; }
    [XmlElement("group")]
    public Group[] groups { get; set; }
    [XmlElement("field")]
    public Field[] Issues { get; set; }`public Field[] Issues { get; set; }

    public bool HasNoIssues
    {
        get
        {
            return Issues == null || Issues.Length == 0;
        }
    }

    public bool HasWarnings
    {
        get
        {
            if (!HasNoIssues)
                return Issues.Where(x => x.name == "Warnings").Count() > 0;
            return false;
        }
    }

    public bool HasErrors
    {
        get
        {
            if (!HasNoIssues)
                return Issues.Where(x => x.name == "Errors").Count() > 0;
            return false;
        }
    }

    public ResponseTags[] Warnings
    {
        get
        {
            if (HasWarnings)
                return Issues.Where(x => x.name == "Warnings").First().groups.First().warnings;
            return new ResponseTags[0];
        }
    }

    public ResponseTags[] Errors
    {
        get
        {
            if (HasErrors)
                return Issues.Where(x => x.name == "Errors").First().groups.First().warnings;
            return new ResponseTags[0];
        }
    }

    public string DisplayWarnings
    {
        get
        {
            var warningString = "";
            if (HasWarnings)
            {
                warningString += Issues.Where(x => x.name == "Warnings").First().groups.First().warnings.Aggregate("", (data, t) => data + t.value +"\n");                   
            }
            return warningString;

        }
    }

    public string DisplayErrors
    {
        get
        {
            var errorString = "";
            if (HasErrors)
            {
                errorString += Issues.Where(x => x.name == "Errors").First().groups.First().errors.Aggregate("", (data, t) => data + t.value+"\n");
            }
            return errorString;

        }
    }}

[Serializable]
public class Segment
{
    [XmlAttribute]
    public string type { get; set; }
    [XmlElement("field")]
    public Field[] fields { get; set; }
}

[Serializable]
public class Group
{
    [XmlAttribute]
    public string type { get; set; }
    [XmlElement("segment")]
    public Segment[] segments { get; set; }
    [XmlElement("e")]
    public ResponseTags[] errors { get; set; }
    [XmlElement("w")]
    public ResponseTags[] warnings { get; set; }
    [XmlElement("i")]
    public ResponseTags[] informations { get; set; }
}


[Serializable]
public class Field
{
    [XmlAttribute]
    public string name { get; set; }
    [XmlText]
    public string value { get; set; }
    [XmlElement("datatype")]
    public DataType datatype { get; set; }
    [XmlElement("group")]
    public Group[] groups { get; set; }
}


[Serializable]
public class DataType
{
    [XmlAttribute]
    public string type { get; set; }
    [XmlElement("field")]
    public Field[] fields { get; set; }
}

[Serializable]
public class ResponseTags
{
    [XmlAttribute]
    public string value { get; set; }
    [XmlAttribute]
    public string path { get; set; }
    [XmlAttribute]
    public string type { get; set; }
    [XmlAttribute]
    public string HL7ErrorCode { get; set; }
    [XmlAttribute]
    public string HL7ErrorCodeFullDescription { get; set; }
    [XmlAttribute]
    public string SegmentID { get; set; }
    [XmlAttribute]
    public string FieldPosition { get;set;}
    [XmlAttribute]
    public string FieldRepetition { get; set; }
    [XmlAttribute]
    public string ComponentNumber { get; set;}
    [XmlAttribute]
    public string SubComponentNumber { get; set;}
    [XmlAttribute]
    public string Severity { get; set; }
}

Then the object is being populated by some xml deserialization

private Message getMessage(string xml)
    {
        if (xml.Length == 0)
            return new Message();
        XmlSerializer serial = new XmlSerializer(typeof(Message));
        MemoryStream soapwriter = new MemoryStream(Encoding.UTF8.GetBytes(xml));
        return (Message)serial.Deserialize(soapwriter);
    }

And here is some test xml, there is no real data in here, I have scraped that out

<message xmlns="" type="VxuV04">
    <segment type="msh">
        <field name="MSH-1 (FieldSeperator)">
            <![CDATA[ | ]]>
        </field>
        <field name="MSH-2 (EncodingCharacters)">
            <![CDATA[ ^~\& ]]>
        </field>
        <field name="MSH-3 (SendingApplication)">
            <datatype type="hd">
                <field name="NamespaceID">OMG IMMUNIZATIO</field>
                <field name="UniversalID"/>
                <field name="UniversalIDType"/>
            </datatype>
        </field>
        <field name="MSH-4 (SendingFacility)">
            <datatype type="hd">
                <field name="NamespaceID">FF0000</field>
                <field name="UniversalID"/>
                <field name="UniversalIDType"/>
            </datatype>
        </field>
        <field name="MSH-5 (ReceivingApplication)">
            <datatype type="hd">
                <field name="NamespaceID">AK TEST FACILITY</field>
                <field name="UniversalID"/>
                <field name="UniversalIDType"/>
            </datatype>
        </field>
        <field name="MSH-6 (ReceivingFacility)">
            <datatype type="hd">
                <field name="NamespaceID">ML1001</field>
                <field name="UniversalID"/>
                <field name="UniversalIDType"/>
                <group type="ProblemCollection">
                    <e value="Invalid value: ML1001. Reason: No user assigned to this facility." path="MSH/ReceivingFacility/NamespaceID" type="problem" HL7ErrorCode="103" HL7ErrorCodeFullDescription="Table value not found" SegmentID="Msh" SegmentSequence="" FieldPosition="6" FieldRepetition="1" ComponentNumber="" SubComponentNumber="" Severity="E"/>
                </group>
            </datatype>
        </field>
        <field name="MSH-7 (DateTimeOfMessage)">3/28/2014</field>
        <field name="MSH-9 (MessageType)">
            <datatype type="msg">
                <field name="MessageCode">VXU</field>
                <field name="TriggerEvent">V04</field>
                <field name="MessageStructure">VXU_V04</field>
            </datatype>
        </field>
        <field name="MSH-10 (MessageControlID)">dsfg00541680</field>
        <field name="MSH-11 (ProcessingID)">
            <datatype type="processingtype">
                <field name="ProcessingID">P</field>
                <field name="ProcessingMode"/>
            </datatype>
        </field>
        <field name="MSH-12 (HL7Version)">2.5.1</field>
        <field name="MSH-15 (AcceptAcknowledgmentType)"/>
        <field name="MSH-16 (ApplicationAcknowledgmentType)"/>
        <field name="MSH-21 (MessageProfileIdentifier)">
            <datatype type="ei">
                <field name="EntityIdentifier"/>
                <field name="NamespaceID"/>
                <field name="UniversalID"/>
                <field name="UniversalIDType"/>
            </datatype>
        </field>
        <field name="MSH-22 (ResponsibleSendingOrganization)">
            <datatype type="xon">
                <field name="OrganizationName"/>
                <field name="OrganizationNameTypeCode"/>
                <field name="AssigningAuthority">
                    <datatype type="hd">
                        <field name="NamespaceID"/>
                        <field name="UniversalID"/>
                        <field name="UniversalIDType"/>
                    </datatype>
                </field>
                <field name="IdentifierTypeCode"/>
                <field name="AssigningFacility">
                    <datatype type="hd">
                        <field name="NamespaceID"/>
                        <field name="UniversalID"/>
                        <field name="UniversalIDType"/>
                    </datatype>
                </field>
                <field name="OrganizationIdentifier"/>
            </datatype>
        </field>
        <field name="MSH-23 (ResponsibleReceivingOrganization)">
            <datatype type="xon">
                <field name="OrganizationName"/>
                <field name="OrganizationNameTypeCode"/>
                <field name="AssigningAuthority">
                    <datatype type="hd">
                        <field name="NamespaceID"/>
                        <field name="UniversalID"/>
                        <field name="UniversalIDType"/>
                    </datatype>
                </field>
                <field name="IdentifierTypeCode"/>
                <field name="AssigningFacility">
                    <datatype type="hd">
                        <field name="NamespaceID"/>
                        <field name="UniversalID"/>
                        <field name="UniversalIDType"/>
                    </datatype>
                </field>
                <field name="OrganizationIdentifier"/>
            </datatype>
        </field>
        <field name="Errors">
            <group type="ProblemCollection">
                <e value="MSH-6 (ReceivingFacility) : Invalid value: zz1001. Reason: No user assigned to this facility." path="MSH/ReceivingFacility/NamespaceID" type="problem" HL7ErrorCode="z3" HL7ErrorCodeFullDescription="Table value not found" SegmentID="Msh" SegmentSequence="" FieldPosition="6" FieldRepetition="1" ComponentNumber="" SubComponentNumber="" Severity="E"/>
            </group>
        </field>
    </segment>  
    <field name="Errors">
        <group type="ProblemCollection">
            <e value="MSH-6 (ReceivingFacility) : Invalid value: ML1001. Reason: No user assigned to this facility." path="MSH/ReceivingFacility/NamespaceID" type="problem" HL7ErrorCode="zzz" HL7ErrorCodeFullDescription="Table value not found" SegmentID="Msh" SegmentSequence="" FieldPosition="6" FieldRepetition="1" ComponentNumber="" SubComponentNumber="" Severity="E"/>
        </group>
    </field>
    <field name="Warnings">
        <group type="ProblemCollection">
            <w value="PID-11 (PatientAddress)-Mailing : County Code  is not in the list of known county codes ." path="PID/PatientAddress" type="problem" HL7ErrorCode="zzz" HL7ErrorCodeFullDescription="Table value not found" SegmentID="Pid" SegmentSequence="" FieldPosition="11" FieldRepetition="" ComponentNumber="" SubComponentNumber="" Severity="W"/>
            <w value="PID-11 (PatientAddress)-Physical : County Code  is not in the list of known county codes ." path="PID/PatientAddress" type="problem" HL7ErrorCode="zzz" HL7ErrorCodeFullDescription="Table value not found" SegmentID="Pid" SegmentSequence="" FieldPosition="11" FieldRepetition="" ComponentNumber="" SubComponentNumber="" Severity="W"/>
        </group>
    </field>
    <field name="ErrorsAndWarningsCombined">
        <group type="ProblemCollection">
            <e value="MSH-6 (ReceivingFacility) : Invalid value: ML1001. Reason: No user assigned to this facility." path="MSH/ReceivingFacility/NamespaceID" type="problem" HL7ErrorCode="zz" HL7ErrorCodeFullDescription="Table value not found" SegmentID="Msh" SegmentSequence="" FieldPosition="6" FieldRepetition="1" ComponentNumber="" SubComponentNumber="" Severity="E"/>
            <w value="PID-11 (PatientAddress)-Mailing : County Code is not in the list of known county codes" path="PID/PatientAddress" type="problem" HL7ErrorCode="zz" HL7ErrorCodeFullDescription="Table value not found" SegmentID="Pid" SegmentSequence="" FieldPosition="11" FieldRepetition="" ComponentNumber="" SubComponentNumber="" Severity="W"/>
            <w value="PID-11 (PatientAddress)-Physical : County Code is not in the list of known county codes ." path="PID/PatientAddress" type="problem" HL7ErrorCode="zzz" HL7ErrorCodeFullDescription="Table value not found" SegmentID="Pid" SegmentSequence="" FieldPosition="z" FieldRepetition="" ComponentNumber="" SubComponentNumber="" Severity="W"/>
        </group>
    </field>
</message>

I promise the closing message tag is there

Answer 1

I think that most things are said, I recapitulate and add some of my own stuff:

• As Jesse said, I'd use Any instead of Where + Count. It should have a better performance, I think.
• Properties are for me: HasNoIssues, HasWarnings and HasErrors
• In my Opinion the rest should be methods
• Issues.Where(x => x.Name == "Warnings").First() is the same as Issues.First(x => x.Name == "Warnings")
• I changed the names of the functions (but it may be a matter of taste).

So it would be in my approach like this:

    public Field[] Issues { get; set; }

    public bool HasErrors
    {
        get
        {
            if (HasIssues)
            {
                return Issues.Any(x => x.Name == "Errors");
            }
            return false;
        }
    }

    public bool HasIssues
    {
        get
        {
            return Issues != null && Issues.Length > 0;
        }
    }

    public bool HasWarnings
    {
        get
        {
            if (HasIssues)
            {
                return Issues.Any(x => x.Name == "Warnings");
            }
            return false;
        }
    }

    public ResponseTags[] GetErrors()
    {
        if (HasErrors)
        {
            return Issues.First(x => x.Name == "Errors").Groups.First().Warnings;
        }
        return new ResponseTags[0];
    }

    public ResponseTags[] GetWarnings()
    {
        if (HasWarnings)
        {
            return Issues.First(x => x.Name == "Warnings").Groups.First().Warnings;
        }
        return new ResponseTags[0];
    }

    // For the display functions:
    // I don't understand why you only usses the first
    // groups of errors and warnings, but that may be a requirement.

    public string GetErrorsDisplayData()
    {
        var errorString = "";
        if (HasErrors)
        {
            errorString += Issues.First(x => x.Name == "Errors")
                .Groups.First()
                .Errors.Aggregate("", (data, t) => data + t.Value + "\n");
        }
        return errorString;
    }

    public string GetWarningsDisplayData()
    {
        var warningString = "";
        if (HasWarnings)
        {
            warningString += Issues.First(x => x.Name == "Warnings")
                .Groups.First()
                .Warnings.Aggregate("", (data, t) => data + t.Value + "\n");
        }
        return warningString;
    }

Answer 2

Methods Set and Get Properties through their Get and Set methods, so it isn't a really good idea to perform operations on properties inside their get and set methods.

you should use the object methods to do this.

public string DisplayErrors
{
    get
    {
        var errorString = "";
        if (HasErrors)
        {
            errorString += Issues.Where(x => x.name == "Errors").First().groups.First().errors.Aggregate("", (data, t) => data + t.value+"\n");
        }
        return errorString;

    }
}

should be like this

public string DisplayErrors (bool inputBoolean)
{
    var errorString = "";
    if (inputBoolean)
    {
         errorString += Issues.Where(x => x.name == "Errors").First().groups.First().errors.Aggregate("", (data, t) => data + t.value+"\n");
    }
    return errorString;
}

Console.WriteLine(DisplayErrors(HasErrors));

DisplayErrors is an action to me, so it should be a method that the object performs not a property of the object,

• Property = description
• Method = Action

---

Look at what is in the HasWarnings property. A boolean property should be an object flag that is set to true or false, a method inside the object can get or set the flag (to either true or false) or it can be visible to something interacting with the object.

All of this makes me think over the whole piece of code, if I create a new object all of these properties are referring to other properties nothing is set.

Warnings property looks at HasWarnings property which in turn looks at HasNoIssues property (which really should be something like HasIssues) inside of a class called Issues , it has Issues.

This isn't a completed, working class, if you develop it farther you should see that this isn't a good design at all

HasWarnings should be a property, but it shouldn't look to another property for it's value.

There needs to be a constructor that sets the basic properties, this is where you say

if (HasIssues) {
    HasWarnings = Issues.Where(x => x.name == "Warnings").Count() > 0;
} else {
    HasWarnings = false;
}

Answer 3

Seeing as Issues is a fixed size array, personally I'd be looking into caching the results (unless Issues or things within the Issues will be changing more than the properties are being called).

Though if your class is being populated with de-serialisation and it's not supposed to be changed (as I'd assume it is given the class' name), I'd be tempted to make it publicly immutable by setting all the setters to private/protected. This would have the additional benefit of meaning you could evaluate the properties at the point of de-serialisation and not have to query the arrays from within properties at all.