HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Parsing an XML file and creating an object with multiple properties from the XML

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromfilethecreatingwithpropertiesxmlparsingmultipleand

Problem

This question contains some C# code I've written as well as a XML format I have designed. My C# code runs a series of "jobs" by loading the XML file and then calling different SQL stored procedures and E-Mailing the results to different addresses. My question specifically relates to the C# code to parse the XML document and also how well designed the XML format is.

The code allows for a variable number of email addresses and stored procedure parameters. To allow for multiple named stored procedure parameters, I'm using a dictionary. The key is the name of the parameter and the value is the value to be used with said parameter.

My questions:

  • How well designed is the XML?



  • How good or bad is the C# code to parse the file?



  • How can it be improved?



  • Is there anything majorly wrong with it?



This is the XML format I'm using (in this case there are only two jobs defined):


  
    
    

    
      
        
        
      

      
        
        
      

      
        
        
      
    
  

  
    
    
    
    

    
      
        
        
      

      
        
        
      

      
        
        
      
    
  


And this is the C# code to parse the XML:

```
public IEnumerable GetJobConfigurations()
{
XDocument jobs = XDocument.Load(@"jobs.xml");

var query = jobs.Descendants("jobService").Descendants("job")
.Select(x => new JobConfiguration
{
Name = x.Attribute("name").Value,
StoredProcedure = x.Attribute("storedProcedure").Value,
ProcedureParameters = x.Elements("procedureParameter").ToDictionary(p => p.Attribute("name").Value, p => p.Attribute("value").Value),
EmailProperties = x.Elements("emailSettings").Select(y => new EmailProperties
{
SubjectName = y.Attribute("subjectName").Value,
AttachmentName = y.Attribute("attachmentName").Value,
SenderAddress = y.Attribute("senderAddress").Value,

Solution

I'm going to put a disclaimer on this. My answer isn't necessarily a better way to handle this particular use case. You seem to have the simplest thing that works as far as your XML schema goes. So, take this with a grain of salt and discuss the pros/cons of each with your colleagues.

public class EmailProperties
{
    public string SenderAddress { get; set; }
    public IList Recipients { get; set; }
    public IList RecipientsBcc { get; set; }
    public IList RecipientsCC { get; set; }
}


This isn't a very object oriented way to do this. What happens if someone should be both "To" and "Bcc"? You'd have to duplicate the email address in the data. I would introduce another class.

public class Recepient
{
    public string EmailAddress { get; set; }
    public bool To { get; set; }
    public bool Cc { get; set; }
    public bool Bcc { get; set; }
}


Which makes your XML look like this.


    
      address@address.com
    
    
      john.doe@domain.com
    
 


Note that the inner and outer elements match, but the outer is plural, while the inner is singular. Also, the value is placed as the value of the element instead of as a property. These are all semantically the same type of thing, but they have different attributes for whether they're "To", "Cc", etc. If it's a business rule that no one can ever have more than one of these, I'd change it a bit.


    John.doe@domain.com


And the corresponding change to the new class.

public enum RecepientType 
{
    To, 
    Cc,
    Bcc
}

public class Recepient
{
    public string EmailAddress { get; set; }
    public RecepientType Type { get; set; }
}


Now, rather than review your parsing code (props for using XDoc by the way, a lot of people try to parse XML the hard way), I'm going to suggest you look at using serialization to do this instead. I won't get into the nitty gritty here, but look into the System.Xml.Serialization namespace. It allows us to work directly with strongly typed objects and doing all that nasty parsing for us.

Code Snippets

public class EmailProperties
{
    public string SenderAddress { get; set; }
    public IList<string> Recipients { get; set; }
    public IList<string> RecipientsBcc { get; set; }
    public IList<string> RecipientsCC { get; set; }
}
public class Recepient
{
    public string EmailAddress { get; set; }
    public bool To { get; set; }
    public bool Cc { get; set; }
    public bool Bcc { get; set; }
}
<recipients>
    <recipient to="true" >
      address@address.com
    <recipient />
    <recipient cc="true">
      john.doe@domain.com
    <recipient  />
 <recipients />
<recipient type="to">
    John.doe@domain.com
<recipient />
public enum RecepientType 
{
    To, 
    Cc,
    Bcc
}

public class Recepient
{
    public string EmailAddress { get; set; }
    public RecepientType Type { get; set; }
}

Context

StackExchange Code Review Q#103853, answer score: 2

Revisions (0)

No revisions yet.