4
\$\begingroup\$

A week ago, I applied for a 'Software Engineering Internship' at one of the company. Since I know C++, C#, Java well and have worked on couple of ASP.NET MVC projects with MS SQL server before, I knew I was well qualified for the job. Plus, I recently graduated from my Bachelor of Computer Science with High Distinction average so this opportunity was exactly what I was looking for.

I sent out my application and was shortlisted for a phone interview. I was really excited and couple of days after I got a phone call from HR and they told me that my skill sets were perfect and they want me to do a practical test before moving on to face to face interview. The lady told me that she was very confident that I would be able to smash the practical test and they would ask some database related questions on the test.

Before the practical test, I started studying SQL a lot, I did all the questions I could find on the internet and was fairly confident. On the day of the practical test, she sent me an email with PDF with the questions.

They told me I had 4 hours to complete those tasks. When I first looked at the questions, I was fairly surprised since I didn't expect the questions to be on this subject. I prepared for SQL. haha I had never programmed Web API before but I had done ASP.NET MVC web applications and knew that we can create API in the controller that can return JSON data which can be consumed by client using AJAX calls.

So, for the first task, I was thinking of using Entity Framework connecting to database file and mapping it to objects but in the second task they mentioned not to use Entity Framework so I had to use something else. At the end, I used XML format for data store because it was easy to hand craft an XML file for debugging purposes.

My Category class looks like this:

[Serializable]
public class Category
{
    public string Name { get; set; }
    public List<Category> Childs { get; set; }

    public Category()
    {
        Childs = new List<Category>();
    }

    public Category(string name)
    {
        Name = name;
        Childs = new List<Category>();
    }
}

The whole idea was that I would create category data structure using that class and use XmlSerializer to serialise and deserialise it to and from a file.

After that, I moved on to task 2, which was to create Web API. I had 2 options here, either use Web API project in Visual Studios to create those APIs or use the MVC Controller class to create them. I have never used Web API before and since I only had limited amount of time to complete the task, I decided to go with the second option. Learning to use WebAPI might have taken another hour and I didn't have that much time, but if I was working without the time limit, I would have definitely gone the WebAPI route.

Here are my API functions that client can consume. I kept it all in HomeContoller.cs file:

public class HomeController : Controller
{
    //Index view, pretty simple one
    public ActionResult Index()
    {
        return View();
    }

    //API that refreshes the category structure, created this just for debugging
    public void RefreshStructure()
    {
        Category category = new Category("root");
        category.Childs.Add(new Category("English"));
        category.Childs.Add(new Category("History"));
        category.Childs.Add(new Category("Science"));
        category.Childs.ElementAt(0).Childs.Add(new Category("Literature"));
        category.Childs.ElementAt(0).Childs.Add(new Category("Language"));
        category.Childs.ElementAt(2).Childs.Add(new Category("Biology"));
        category.Childs.ElementAt(2).Childs.Add(new Category("Chemistry"));
        category.Childs.ElementAt(2).Childs.Add(new Category("Physics"));
        saveAllCategories(category);
    }

    //Creates root category from XML file
    private Category getAllCategories()
    {
        XmlSerializer xmlS = new XmlSerializer(typeof(Category));
        FileStream readFileStream = new FileStream(Server.MapPath("~/datamodel/model.xml"), FileMode.Open, FileAccess.Read, FileShare.Read);
        Category category = (Category)xmlS.Deserialize(readFileStream);
        readFileStream.Close();
        return category;
    }

    //Save Categories to XML file
    private void saveAllCategories(Category categories)
    {
        XmlSerializer xmlS = new XmlSerializer(typeof(Category));
        TextWriter textWriter = new StreamWriter(Server.MapPath("~/datamodel/model.xml"));
        xmlS.Serialize(textWriter, categories);
        textWriter.Close();
    }

    //Navigates to particulate category
    private Category navigateToCategory(IEnumerable<string> path, Category root)
    {
        Category destination = root;

        if (path.Count() == 1)  //It has got to be the root
        {
            return root;
        }
        else
        {
            for (int i = 1; i < path.Count(); ++i)
            {
                destination = destination.Childs.FirstOrDefault(c => c.Name == path.ElementAt(i));
                if (destination == null)    //Opps that is a problem, path doesn't exist
                {
                    return null;
                }
            }
        }
        return destination;
    }

    //Creates ouput that can be sent as Json. Categories name separated by ~
    private string getDelimiterSeparatedCategories(Category category)
    {
        string retVal = category.Name;
        foreach (var c in category.Childs)
        {
            retVal += "~" + c.Name;
        }
        return retVal;
    }

    //API to create Category
    [HttpPost]
    public JsonResult CreateCategory(IEnumerable<string> path, string name)
    {
        Category root = getAllCategories();
        Category destination = navigateToCategory(path, root);
        if (destination == null)
        {
            return Json(new { Result = "ERROR" });
        }

        destination.Childs.Add(new Category(name));
        string retVal = getDelimiterSeparatedCategories(destination);
        saveAllCategories(root);
        return Json(new { Result = "OK", Data =  retVal});
    }

    //API to retrieve category
    [HttpPost]
    public JsonResult RetrieveCategories(IEnumerable<string> path)
    {
        Category root = getAllCategories();
        Category destination = navigateToCategory(path, root);
        if (destination == null)
        {
            return Json(new { Result = "ERROR" });
        }

        string retVal = getDelimiterSeparatedCategories(destination);
        return Json(new { Result = "OK", Data = retVal });
    }

    //API to update category
    [HttpPost]
    public JsonResult UpdateCategory(IEnumerable<string> path, string name)
    {
        if (path.Count() == 1)
        {
            return Json(new { Result = "ERROR" });
        }

        Category root = getAllCategories();
        Category destination = navigateToCategory(path, root);
        if (destination == null)
        {
            return Json(new { Result = "ERROR" });
        }

        destination.Name = name;
        string retVal = getDelimiterSeparatedCategories(destination);
        saveAllCategories(root);
        return Json(new { Result = "OK", Data = retVal });
    }

    //API to delete category
    [HttpPost]
    public JsonResult DeleteCategory(IEnumerable<string> path)
    {
        if (path.Count() == 1)
        {
            return Json(new { Result = "ERROR" });
        }

        List<string> pathParent = new List<string>();

        for (int i = 0; i < path.Count() - 1; ++i)
        {
            pathParent.Add(path.ElementAt(i));
        }

        Category root = getAllCategories();
        Category parent = navigateToCategory(pathParent.AsEnumerable(), root);
        Category destination = navigateToCategory(path, root);

        if (destination == null)
        {
            return Json(new { Result = "ERROR" });
        }

        parent.Childs.Remove(destination);
        saveAllCategories(root);
        string retVal = getDelimiterSeparatedCategories(parent);
        return Json(new { Result = "OK", Data = retVal });
    }
}

As you can see, I have 4 API exposed for client to consume:

  • CreateCategory
  • RetrieveCategories
  • UpdateCategory
  • DeleteCategory

After finishing those, I went on to create the client that could consume those APIs:

@{
    ViewBag.Title = "Home Page";
}

<h1 id="CurrentCategory"></h1>

<div id="CategoriesList"></div>

<input type="text" placeholder="Add Category" id="CategoryName" /><button onclick="addCategory()">Add Category</button><br />
<input type="text" placeholder="Update Category" id="NewName" /><button onclick="updateCategory()">Update Category</button><br />
<button onclick="deleteCategory()">Delete Category</button>

@section scripts {
<script type="text/javascript">
    var cList = document.getElementById("CategoriesList");
    var cCategory = document.getElementById("CurrentCategory")
    var pArray = new Array();
    pArray[0] = "root";
    retrieveCategories();

    function retrieveCategories() {
        $.ajax({
            url: '@Url.Action("RetrieveCategories", "Home")',
            type: 'POST',
            datatype: 'json',
            data: { path: pArray },
            success: function (data) {
                var categories = getCategoriesList(data.Data);
                var list = "<ul>";
                cCategory.innerHTML = categories[0];
                for (var i = 1; i < categories.length; ++i) {
                    list += "<li><a href=\"javascript:gotoParticularCategories('" + categories[i] + "')\">" + categories[i] + "</a></li>";
                }
                list += "</ul>";
                cList.innerHTML = list;
            },
            error: function () {
                cList.innerHTML = "There was an error! Please refresh the page";
            }
        });
    }

    function getCategoriesList(input) {
        var retVal = input.split("~");
        return retVal;
    }

    function gotoParticularCategories(category) {
        pArray[pArray.length] = category;
        retrieveCategories();
    }

    function deleteCategory() {
        $.ajax({
            url: '@Url.Action("DeleteCategory", "Home")',
            type: 'POST',
            datatype: 'json',
            data: { path: pArray },
            success: function (data) {
                if (data.Result != "ERROR") {
                    pArray.splice(pArray.length - 1, 1);
                }
                var categories = getCategoriesList(data.Data);
                var list = "<ul>";
                cCategory.innerHTML = categories[0];
                for (var i = 1; i < categories.length; ++i) {
                    list += "<li><a href=\"javascript:gotoParticularCategories('" + categories[i] + "')\">" + categories[i] + "</a></li>";
                }
                list += "</ul>";
                cList.innerHTML = list;
            },
            error: function () {
                cList.innerHTML = "There was an error! Please refresh the page";
            }
        });
    }

    function addCategory() {
        var categoryName = document.getElementById("CategoryName").value;
        $.ajax({
            url: '@Url.Action("CreateCategory", "Home")',
            type: 'POST',
            datatype: 'json',
            data: { path: pArray, name: categoryName },
            success: function (data) {
                var categories = getCategoriesList(data.Data);
                var list = "<ul>";
                cCategory.innerHTML = categories[0];
                for (var i = 1; i < categories.length; ++i) {
                    list += "<li><a href=\"javascript:gotoParticularCategories('" + categories[i] + "')\">" + categories[i] + "</a></li>";
                }
                list += "</ul>";
                cList.innerHTML = list;
            },
            error: function () {
                cList.innerHTML = "There was an error! Please refresh the page";
            }
        });
    }

    function updateCategory() {
        var categoryName = document.getElementById("NewName").value;
        $.ajax({
            url: '@Url.Action("UpdateCategory", "Home")',
            type: 'POST',
            datatype: 'json',
            data: { path: pArray, name: categoryName },
            success: function (data) {
                var categories = getCategoriesList(data.Data);
                var list = "<ul>";
                cCategory.innerHTML = categories[0];
                for (var i = 1; i < categories.length; ++i) {
                    list += "<li><a href=\"javascript:gotoParticularCategories('" + categories[i] + "')\">" + categories[i] + "</a></li>";
                }
                list += "</ul>";
                cList.innerHTML = list;
            },
            error: function () {
                cList.innerHTML = "There was an error! Please refresh the page";
            }
        });
    }
</script>    
}

I know my code could have been structured so much better but I was really trying to finish this in time and if I had time I would have refactored it so it looked much better. For the client I am using AJAX calls to call the APIs I had created before.

At the end, I had a functional web application that did everything they asked for and I sent it to them on time, but I am concerned that it might not have been enough. I feel like I could have done a lot better and I am really passionate about programming and really want this internship. It's been 2 days already and they haven't contacted me and I am really stressed out.

I just wanted to ask if you think I did enough to pass through it or not.

If you want to download the project and play around with it, it's here.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$
[Serializable]
public class Category
{
    public string Name { get; set; }
    public List<Category> Childs { get; set; }

    public Category()
    {
        Childs = new List<Category>();
    }

    public Category(string name)
    {
        Name = name;
        Childs = new List<Category>();
    }
}

Constructor chaining is your friend which helps you to reduce complexity and (usually) the amount of code.

[Serializable]
public class Category
{
    public string Name { get; set; }
    public List<Category> Childs { get; set; }

    public Category()
    {
        Childs = new List<Category>();
    }

    public Category(string name)
        : this()
    {
        Name = name;
    }
}  

but moreover you should ask yourself, do I really want any outside caller to set the Childs property or would it be enough to access/change/delete the items of that List<T>. If the latter is enough, make the setter private.


HomeController

//Creates root category from XML file
private Category getAllCategories()
{
    XmlSerializer xmlS = new XmlSerializer(typeof(Category));
    FileStream readFileStream = new FileStream(Server.MapPath("~/datamodel/model.xml"), FileMode.Open,  FileAccess.Read, FileShare.Read);
    Category category = (Category)xmlS.Deserialize(readFileStream);
    readFileStream.Close();
    return category;
}
  • The FileStream implements IDisposable hence it should be enclosed in a using block which takes care of disposing the FileStream object and therefor closing the underlying stream.

  • What happens if the file doesn't exist ? You should enclose it in a try..catch.


private void saveAllCategories(Category categories)
{
    XmlSerializer xmlS = new XmlSerializer(typeof(Category));
    TextWriter textWriter = new StreamWriter(Server.MapPath("~/datamodel/model.xml"));
    xmlS.Serialize(textWriter, categories);
    textWriter.Close();
}

The TextWriter implements the IDisposable interface as well.


//Navigates to particulate category
private Category navigateToCategory(IEnumerable<string> path, Category root)
{
    Category destination = root;

    if (path.Count() == 1)  //It has got to be the root
    {
        return root;
    }
    else
    {
        for (int i = 1; i < path.Count(); ++i)
        {
            destination = destination.Childs.FirstOrDefault(c => c.Name == path.ElementAt(i));
            if (destination == null)    //Opps that is a problem, path doesn't exist
            {
                return null;
            }
        }
    }
    return destination;
}  

The else is redundant, because it won't be reached if path.Count() == 1.

If you store the result of path.Count() into a variable it can be reused, right now you calculate this for each item in the IEnumerable<T>. If that IEnumerable<T> would be some type of ICollection<T> that wouldn't do that much harm, just a cast and accessing the Count property, but for any other underlying type, it would need to iterate over the whole items and count them.


//Creates ouput that can be sent as Json. Categories name separated by ~
private string getDelimiterSeparatedCategories(Category category)
{
    string retVal = category.Name;
    foreach (var c in category.Childs)
    {
        retVal += "~" + c.Name;
    }
    return retVal;
}  

Never ever use string concatenation within a loop, thats what a StringBuilder is for.

If you would override the ToString() method of the Category class like so

public override string ToString()
{
    return Name;
}  

you could take advantage of the string.Join() method like so

private string getDelimiterSeparatedCategories(Category category)
{
    return category.Name + "~" + string.Join("~", category.Childs);
}  

//API to delete category
[HttpPost]
public JsonResult DeleteCategory(IEnumerable<string> path)
{
    if (path.Count() == 1)
    {
        return Json(new { Result = "ERROR" });
    }

    List<string> pathParent = new List<string>();

    for (int i = 0; i < path.Count() - 1; ++i)
    {
        pathParent.Add(path.ElementAt(i));
    }

    Category root = getAllCategories();
    Category parent = navigateToCategory(pathParent.AsEnumerable(), root);
    Category destination = navigateToCategory(path, root);

    if (destination == null)
    {
        return Json(new { Result = "ERROR" });
    }

    parent.Childs.Remove(destination);
    saveAllCategories(root);
    string retVal = getDelimiterSeparatedCategories(parent);
    return Json(new { Result = "OK", Data = retVal });
}

I don't get why you have this List<string> pathParent in there. A much better way would be to use the Take() extension method. Another thing to mention is that you should retrieve the parent after you have checked the destination against null. If destination is null then there is no reason to retrieve the parent.

//API to delete category
[HttpPost]
public JsonResult DeleteCategory(IEnumerable<string> path)
{
    int pathCount = path.Count();

    if (pathCount == 1)
    {
        return Json(new { Result = "ERROR" });
    }

    Category root = getAllCategories();

    Category destination = navigateToCategory(path, root);

    if (destination == null)
    {
        return Json(new { Result = "ERROR" });
    }

    Category parent = navigateToCategory(path.Take(pathCount - 1), root);

    parent.Childs.Remove(destination);

    saveAllCategories(root);
    string retVal = getDelimiterSeparatedCategories(parent);

    return Json(new { Result = "OK", Data = retVal });
}

As a side note, because List<T> implements IEnumerable<T> there is no need to call AsEnumerable() on such a List<T> if you need an IEnumerable<T>.


Style

  • I don't like the camelCase casing for naming private methods. Why would you want to distinguish between private and public by the casing style? If you later decide to change a private method to public you will likely forget to rename the method.

  • The comments above the methods like //Navigates to particulate category don't add any value. If you would have proper xml documentation this would also show up in intellisense. That being said, for public methods which are part of an API proper xml documentation is a must-have.

  • you are using braces {} although they might be optional which is good.

  • you should consider to change some method names; e.g

    Category root = getAllCategories();  
    

    is just misleading. Maybe GetRootCategory would be a better name.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ +1 but you don't need to override ToString to use Join: string.Join("~", category.Childs.Select(c => c.Name)) would work too (in .Net4 or above). \$\endgroup\$
    – RobH
    Commented Nov 18, 2015 at 9:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.