Iterate through a list of data and add it to nested models

Question

I have to go through files with metadata and I am unhappy with the iteration and how I have build it due to the fact, that I have a modeltree with lists. So basically I iterate through filesOfFolder and pick each file and then I am adding the metadata. In the else I am checking, if the fileName has the same beginning then the fileNameBefore, because then it is a child of the last file, and it needs a different documentOrderNumber.

The modeltree is like and it cannot get changed:

Mainmodel
   Documents (public MainDocumentModel[] and AttDocumentModel[])
       MainDocumentModel
           FileContent
       AttDocumentModel
           FileContent

So as of this tree I did it so and I am totally unhappy with that:

string fileNameBefore = "";
model.Documentlist = new Documents
{
    Attdocument = new AttDocumentModel[filesOfFolder.Count() + 1]
};
int i = 0;
int j = 0;
int k = 0;

string mainPath = System.Web.HttpContext.Current.ApplicationInstance.Server.MapPath("~/App_Data/" + model.DocId + "/");
string directoryAttPath = mainPath + "Att/";
Directory.CreateDirectory(directoryAttPath);

foreach (var file in filesOfFolder)
{
    if (file.f_name.Contains("Maindocument") || file.f_name.Contains("Wrapdocument"))
    {
        FileContent fContent = new FileContent();
        fContent.FileType = "PDF";
        fContent.FileName = "Maindocument.pdf";

        MainDocumentModel mainDoc = new MainDocumentModel();
        mainDoc.MainDoc = new List<FileContent>();
        mainDoc.Message = "";
        mainDoc.Status = true;
        mainDoc.MainDoc.Add(fContent);

        model.Documentlist.Maindocument = new MainDocumentModel[1];
        model.Documentlist.Maindocument[0] = mainDoc;
        string mainDocumentPath = mainPath + fContent.FileName;
        System.IO.File.WriteAllBytes(mainDocumentPath, file.f_file);
    }
    else
    {
        if (!(fileNameBefore.Split(' ')[0] == file.f_name.Split(' ')[0]))
        {
            j = 0;
            i++;
        }
        else
        {
            j++;
        }
        FileContent attachment = new FileContent();
        string documentOrderNumber = GenerateOrderNumberForRis(i, j, fileNameBefore);
        attachment.FileName = documentOrderNumber + ".pdf";
        attachment.FileType = "PDF";

        AttDocumentModel attDoc = new AttDocumentModel();
        attDoc.AttDoc = new List<FileContent>();
        attDoc.Message = "";
        attDoc.Status = true;
        attDoc.ShowName = file.f_subject;
        attDoc.AttDoc.Add(attachment);

        model.Documentlist.Attdocument[k] = attDoc;
        System.IO.File.WriteAllBytes(directoryAttPath + documentOrderNumber + ".pdf", file.f_file);
        k++;
        fileNameBefore = file.f_name;
    }
}

Here are the models:

public class MainModel
{
    ....
    [XmlIgnore]
    public MainDocumentModel Maindocument { get; set; }

    [XmlIgnore]
    public AttDocumentModel Attdocument { get; set; }

    [XmlElement(ElementName = "documentlist")]
    public Documents Documentlist  { get; set; }
}


[XmlRoot(elementName: "maindocument")]
public class MainDocumentModel
{
    [XmlIgnore]
    public String Message { get; set; }

    [XmlIgnore]
    public Boolean Status { get; set; }

    [XmlElement("content")]
    public List<FileContent> MainDoc { get; set; }
}

[XmlRoot(elementName: "attacheddocument")]
public class AttDocumentModel
{
    [XmlIgnore]
    public String Message { get; set; }

    [XmlIgnore]
    public Boolean Status { get; set; }

    [XmlElement("showname")]
    public string ShowName { get; set; }

    [XmlElement("content")]
    public List<FileContent> AttDoc { get; set; }
}

public class FileContent
{
    [XmlElement("dataformat")]
    public string FileType { get; set; }

    [XmlElement("uri")]
    public string FileName { get; set; }
}

Answer 1

Some quick remarks.

• Don't pointlessly abbreviate: fContent isn't a clear name, for instance.

• i, j and k are okay names for variables in a for-loop, but here they really should have meaningful names.

• Avoid adding words like "list" to names of collections. Especially when Documentlist isn't a collection, but the oddly named Documents. A class with a plural name? Which is then initialized by setting the badly named property Attdocument?

• I'm assuming that you cannot change the property name f_name?

• Are you certain that .Contains("Maindocument") will always work? Or should it be case insensitive?

• In several places you seem to use string concatenation to create directory paths. Avoid this and instead use Path.Combine().

• When you construct the MainDocumentModel, try to keep it succinct. (I have omitted Message = "" since IMHO it seems superfluous: why not simply leave it null?) Same for AttDocumentModel in the else, BTW.

    var mainDocumentModel = new MainDocumentModel
    {
        Status = true,
        MainDoc = new List<FileContent>{ fContent }
    };
  

• I have trouble figuring out what if (!(fileNameBefore.Split(' ')[0] == file.f_name.Split(' ')[0])) is supposed to mean. As a matter of fact, that whole if...else seems to reference some kind of logic that really should be documented there with comments. It doesn't help that we don't have access to the code for GenerateOrderNumberForRis.

• There are two places where you do documentOrderNumber + ".pdf".

• Maindocument, Attdocument, ... are compound words and thus should be PascalCase.