Generic ASP.NET MVC controller that generates many pages

Question

I recently set out to create an open-source ASP.net MVC web development framework. Specifically, I wanted to automate some of the tasks associated with the creation of data-driven applications. I've had to create a number of CRUD screens over the years and wanted to automate as many aspects of these screens as possible.

Today, I published an early release of this framework on GitHub and gave it the name "Express Forms".

One of the core parts of Express Forms is a Controller class that takes two type parameters. The first type parameter tells Express Forms what class represents a data record to work with, and the second is meant to specify a type that represents an Id, though at the moment it is hard-coded in many places to be a 32-bit integer.

I have included the code to the generic Controller class below.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.Mvc;
using ExpressForms.Buttons;
using ExpressForms.Inputs;
using System.Reflection;

namespace ExpressForms
{
    public class ExpressFormsController<T, TId> : Controller
        where T : class, new()
    {        
        protected IExpressFormsExchange<T, TId> Exchange { get; set; }

        public ExpressFormsController() 
        {
            // Set default values to properties
            IndexViewName = "ExpressFormsIndex";
            EditorViewName = "ExpressFormsEditor";
            CustomPropertyNames = new Dictionary<string, string>();
            CustomPropertyDisplay = new Dictionary<string, Func<dynamic, string>>();
            CustomEditorInputs = new Dictionary<string, ExpressFormsInput>();
            IgnoredPropertyNames = new string[] { };            
        }

        protected void Initialize(IExpressFormsExchange<T, TId> exchange)
        {
            Exchange = exchange;                        
        }

        #region properties and functions that are used to customize the appearance and behavior of forms
        protected string FormName { get { return typeof(T).Name; } }

        protected virtual ExpressFormsButton[] IndexButtons
        {
            get
            {
                throw new NotImplementedException();
            }
        }
        /// <summary>
        /// A virtual helper function that is meant to be used to get the buttons to display on the editor form.
        /// May be overridden in a derived class to change what buttons appear.
        /// </summary>
        /// <param name="model"></param>        
        /// <param name="isNew"></param>
        protected virtual ExpressFormsButton[] GetEditorButtons(bool isNew)
        {
            List<ExpressFormsButton> buttons = new List<ExpressFormsButton>()
            {
                new ExpressFormsModifyDataButton()
                {
                    // If the user is inserting a new record, show the user an [Insert] button.    
                    IsVisible = isNew,
                    Text = "Save",
                    FormName = FormName,
                    ActionType = ExpressFormsModifyDataButton.ActionTypeEnum.Insert,
                    PostUrl = Url.Action("Postback"),
                    PostType = ExpressFormsModifyDataButton.PostTypeEnum.Ajax
                },
                new ExpressFormsModifyDataButton()
                {
                    // If the user is updating an existing record, show the user an [Update] button.
                    IsVisible = !isNew,
                    Text = "Save",
                    FormName = FormName,
                    ActionType = ExpressFormsModifyDataButton.ActionTypeEnum.Update,
                    PostUrl = Url.Action("Postback"),
                    PostType = ExpressFormsModifyDataButton.PostTypeEnum.Ajax
                }                
            };
            return buttons.ToArray();
        } // end GetEditorButtons

        // A virtual helper function that is meant to be used to get the inputs that appear on the form.
        // May be overridden in a derived class to customize how the form works.
        protected virtual Dictionary<string, ExpressFormsInput> GetEditorInputs(T record)
        {
            // t.GetType() is used here rather than typeof(T) in order to get the most specific type implemented by the object passed in.
            IEnumerable<PropertyInfo> properties = record.GetType().GetProperties()
                .Where(p => (IgnoredPropertyNames==null) || !IgnoredPropertyNames.Contains(p.Name));

            Func<PropertyInfo, ExpressFormsInput> InputSelector = p => GetEditorInput(record, p);

            Dictionary<string, ExpressFormsInput> inputs = properties
                .ToDictionary(p => p.Name, InputSelector);

            return inputs;
        }

        private ExpressFormsInput GetEditorInput(T record, PropertyInfo property)
        {
            ExpressFormsInput input;

            string inputName = property.Name;
            string value = Convert.ToString(property.GetValue(record, null));
            bool isVisible = true;

            input = GetCustomEditorInput(inputName, value, isVisible);

            if (input == null) // we didn't get an input from GetCustomEditorInput
            {
                switch (property.PropertyType.Name)
                {
                    case "Boolean":
                        input = new ExpressFormsCheckBox()
                        {
                            FormName = FormName,
                            InputName = inputName,
                            Value = value,
                            IsVisible = isVisible,
                        };
                        break;
                    default:
                        input = new ExpressFormsTextBox()
                        {
                            FormName = FormName,
                            InputName = inputName,
                            Value = value,
                            IsVisible = isVisible
                        };
                        break;
                }
            }

            // If this property has an associated "CustomPropertyName", use that for the display name.  Otherwise, use the inputName.
            input.InputDisplayName = CustomPropertyNames.Keys.Contains(input.InputName) ? CustomPropertyNames[input.InputName] : input.InputName;

            return input;
        }

        protected virtual ExpressFormsInput GetCustomEditorInput(string inputName, string value, bool isVisible)
        {
            ExpressFormsInput customInput;            

            // If there is a custom input with matching inputName, assign it the value of the input passed in and return it.
            if (CustomEditorInputs.Keys.Contains(inputName))
            {
                customInput = CustomEditorInputs[inputName];
                customInput.Value = value;
                return customInput;
            }
            // Otherwise, return null.
            else            
                return null;
        }

        protected Dictionary<string, ExpressFormsInput> CustomEditorInputs { get; set; }

        protected string IndexTitle { get; set; }        
        protected Dictionary<string, string> CustomPropertyNames { get; set; }
        protected IEnumerable<string> IgnoredPropertyNames { get; set; }
        protected Dictionary<string, Func<dynamic, string>> CustomPropertyDisplay { get; set; }

        protected string IndexViewName { get; set; }
        protected string EditorViewName { get; set; }

        #endregion

        #region public methods that render views
        /// <summary>
        /// Returns a ViewResult to display an "Index" view from which the user may select a row to edit (or view details that may be hidden)
        /// </summary>
        /// <returns></returns>
        public virtual ActionResult Index()
        {
            ExpressFormsIndexViewModel model = new ExpressFormsIndexViewModel()
            {
                RecordType = typeof(T),
                Title = IndexTitle == null ? this.GetType().Name : IndexTitle,
                CustomIndexHeaders = CustomPropertyNames,
                CustomPropertyDisplay = CustomPropertyDisplay,

                Records = Exchange.Get().ToArray()
            };

            return View(IndexViewName, model);
        }

        /// <summary>
        /// Returns a ViewResult to display an "Editor" form from which the user can insert or update data.
        /// </summary>
        /// <param name="id">the ID of the row to update; if null, the user may insert a new row.</param>        
        public virtual ActionResult Editor(TId id)
        {            
            T record = (id == null) ? new T() : Exchange.Get(id);
            bool isNew = id == null;

            ExpressFormsEditorModel model = new ExpressFormsEditorModel()
            {
                Record = record,
                IsNew = isNew,
                Buttons = GetEditorButtons(isNew),
                Inputs = GetEditorInputs(record)
            };

            return View(EditorViewName, model);
        }
        #endregion                

        #region methods that modify the data when called

        /// <summary>
        /// Postback is a single point of entry for the client to update server-side data.
        /// This method may be called via either form post or AJAX as the page designates.        
        /// Declared virtual so that other developers may specify alternative implementation.
        /// </summary>
        /// <param name="record">the record that the user wants to insert/update/delete</param>
        /// <param name="actionType">'INSERT', 'UPDATE', or 'DELETE'</param>
        /// <param name="postType">'AJAX' or 'FORM' to tell the server how to respond</param>
        /// <returns></returns>
        [ValidateInput(false)]
        public virtual ActionResult Postback(T record, string actionType, string postType)
        {
            // Check that a valid postType was specified
            if (postType == null || !new []{"AJAX", "FORM"}.Contains(postType.ToUpper()))
                throw new ArgumentOutOfRangeException("Must specify 'AJAX' or 'FORM' for postType, encountered: " + postType);

            // Check that a valid action was specified.
            if (actionType == null || !new[] { "INSERT", "UPDATE", "DELETE" }.Contains(actionType.ToUpper()))
                throw new ArgumentOutOfRangeException("Must specify 'INSERT', 'UPDATE', or 'DELETE' for actionType, encountered: " + actionType);

            OperationResult result = null;
            switch (actionType.ToUpper())
            {
                case "INSERT":
                    result = Insert(record);
                    break;
                case "UPDATE":
                    result = Update(record);
                    break;
                case "DELETE":
                    // TODO: This depends on the ID field being called "Id".  This needs to be fixed so that the ID is properly looked up.
                    TId id = (TId)(((dynamic)(record)).Id);
                    result = Delete(id);
                    break;
            }

            switch(postType.ToUpper())
            {
                case "AJAX":
                    return Json(result);                    
                case "FORM":
                    return Redirect(Request.UrlReferrer.ToString());                                
                default:
                    throw new InvalidOperationException();
            }
        } // End Postback

        private OperationResult Insert(T record)
        {
            TId id = Exchange.Insert(record);
            return new OperationResult()
            {
                Result = "Insert OK",
                Id = id
            };
        }

        private OperationResult Update(T record)
        {
            Exchange.Update(record);
            return new OperationResult()
            {
                Result = "Update OK",
            };
        }

        private OperationResult Delete(TId Id)
        {
            Exchange.Delete(Id);
            return new OperationResult()
            {
                Result = "Delete OK",
            };
        }

        private class OperationResult
        {
            public string Result { get; set; }
            public TId Id { get; set; }
        }

        #endregion methods that modify the data when called
    }
}

The generic Controller class is meant to be inherited by a class that specifies the type of data record. There are also a number of protected properties that can be modified in a derived class to modify the resulting ViewModel object, and thus make changes to what appears in the user's browser. The data is expected to be fetched using the IExpressFormsExchange interface. (An implementation for Entity Framework is provided.)

The generic Controller has three public methods: Index, Editor, and Postback. The first two provide views for the user to view and edit data. Postback allows the browser to post data to be updated and returns JSON.

There are examples that can be run bundled in the project. I plan to host them on my personal web site soon.

---

This generic Controller class is the bedrock of the web development framework I'm developing, so I'm posting here for thoughts and ideas about what I've done right, and how I may improve it.

One more thing about the functionality. As you may notice, there is a default name for the views to be used. These names match files provided by the framework that provide "generic" views, but a developer using this controller can specify any view that uses the same ViewModel class.

Here is the generic Entity Framework Controller class.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.Mvc;
using System.Data.Objects;
using System.Data.Objects.DataClasses;
using ExpressForms.Inputs;

namespace ExpressForms.Entities
{
    public abstract class EntityController<TEntity> : ExpressForms.ExpressFormsController<TEntity, int?>
        where TEntity : EntityObject, new()
    {
        public EntityController()
            : base()
        {            
            IgnoredPropertyNames = new[] { "EntityState", "EntityKey" };            
        }

        protected void Initialize(ObjectContext objectContext)
        {            
            EntityExchange<TEntity, int?> exchange = EntityExchangeFactory.GetEntityExchange<TEntity, int?>(objectContext);

            Initialize(exchange);
        }        
    }
}

The above Controller comes with the framework. The code below inherits this class and provides a data source:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.Mvc;
using ExpressForms;
using ExpressForms.Entities;
using System.Data.Objects.DataClasses;

namespace ExpressFormsExample.Controllers
{
    /// <summary>
    /// This abstract class inherits the EntityController class and provides the code to initialize the Controller
    /// with the appropriate Entity Framework reference.    
    /// </summary>
    /// <typeparam name="T"></typeparam>
    public abstract class BaseController<T> : ExpressForms.Entities.EntityController<T>
        where T : EntityObject, new()  
    {       
        protected ExpressFormsExampleEntities db;

        public BaseController()
        {
            db = new ExpressFormsExampleEntities();
            Initialize(db);
        }

        ~BaseController()
        {
            db.Dispose();
        }
    }
}

With the above Controller class in the web project, the only thing that you need do to spin up CRUD screens for any record that comes from the data source is to write a Controller that inherits from the BaseController and specifies the generic parameter like so:

using System.Web.Mvc;

namespace ExpressFormsExample.Controllers
{
    public class Engineer0Controller : BaseController<Engineer>
    {        
    }
}

That's a lot of code to post here; if you want to see more code, please visit the link to the project :-)

Answer 1

This switch should be an if statement instead.

        if (input == null) // we didn't get an input from GetCustomEditorInput
        {
            switch (property.PropertyType.Name)
            {
                case "Boolean":
                    input = new ExpressFormsCheckBox()
                    {
                        FormName = FormName,
                        InputName = inputName,
                        Value = value,
                        IsVisible = isVisible,
                    };
                    break;
                default:
                    input = new ExpressFormsTextBox()
                    {
                        FormName = FormName,
                        InputName = inputName,
                        Value = value,
                        IsVisible = isVisible
                    };
                    break;
            }
        }

so it should look like this instead

if (input == null) // we didn't get an input from GetCustomEditorInput
{
    if (property.PropertyType.Name == "Boolean") {
        input = new ExpressFormsCheckBox()
            {
                FormName = FormName,
                InputName = inputName,
                Value = value,
                IsVisible = isVisible,
            };
    } else {
        input = new ExpressFormsTextBox()
            {
                FormName = FormName,
                InputName = inputName,
                Value = value,
                IsVisible = isVisible
            };
    }
}

I indented the multi-lined declaration so that it was more clear to what is going on there (more readable)

---

Your else statements should match the if statement that they are attached to, an example from your code

protected virtual ExpressFormsInput GetCustomEditorInput(string inputName, string value, bool isVisible)
{
    ExpressFormsInput customInput;            

    // If there is a custom input with matching inputName, assign it the value of the input passed in and return it.
    if (CustomEditorInputs.Keys.Contains(inputName))
    {
        customInput = CustomEditorInputs[inputName];
        customInput.Value = value;
        return customInput;
    }
    // Otherwise, return null.
    else            
        return null;
}

I would write the else statement like this

} else {
    return null;
}

there is no need for the comment on what the else statement is doing because it is transparent.

you should declare variables with the least amount of scope possible, so you would create your customInput inside the if statement like this

protected virtual ExpressFormsInput GetCustomEditorInput(string inputName, string value, bool isVisible)
{
    // If there is a custom input with matching inputName, assign it the value of the input passed in and return it.
    if (CustomEditorInputs.Keys.Contains(inputName))
    {
        ExpressFormsInput customInput;
        customInput = CustomEditorInputs[inputName];
        customInput.Value = value;
        return customInput;
    } else {            
        return null;
    }
}

I was a little confused here for a minute, you create another custom input when you already have a dictionary of custom editor inputs. I think that you should be assigning the custom input and returning the key to that custom input instead of actually returning a new custom input. doing it this way would help out with your GetEditorInput method greatly, it would allow you to return from inside the if (input == null) block, except you would take and write it something like this:

string inputName = property.Name;
string value = Convert.ToString(property.GetValue(record, null));
bool isVisible = true;
var CustomEditorInputIndex = GetCustomEditorInput(inputName, value, isVisible) 

if (CustomEditorInputIndex == null) { input = null; }

if (input == null) // we didn't get an input from GetCustomEditorInput
{
    if (property.PropertyType.Name == "Boolean") {
        input = new ExpressFormsCheckBox()
            {
                FormName = FormName,
                InputName = inputName,
                Value = value,
                IsVisible = isVisible,
            };
    } else {
        input = new ExpressFormsTextBox()
            {
                FormName = FormName,
                InputName = inputName,
                Value = value,
                IsVisible = isVisible
            };
    }
    input.inputDisplayName =  input.InputName;
} else {
    input = CustomEditorInputs[CustomEditorInputIndex];
    input.inputDisplayName = CustomPropertyNames[input.InputName];
}
return input;

I changed the return of the GetCustomEditorInput, the name should be changed to something like getCustomEditorInputKey and CustomEditorInputIndex should be something like customEditorInputKey

You should probably do away with the isVisble parameter for that too because you don't use it.

The isVisible variable in the GetEditorInput is not needed in my opinion, you use it twice and it isn't ever set anywhere but the declaration, the argument could be made that it is a magic value so I will leave it there. I haven't rewritten GetCustomEditorInput for you, I will let you have that fun.

---

@BenVlodgi mentioned the use of ternary statements to make this a little cleaner

so what I did was take the nested if statement and turn it into a ternary statement, I am sure that it doesn't make much of a difference to the compiler but makes it a little easier to read, in my opinion.

if (input == null)
{
    input = property.PropertyType.Name == "Boolean" 
        ? new ExpressFormsCheckBox() {FormName = FormName, InputName = inputName, Value = value, IsVisible = isVisible} 
        : new ExpressFormsTextBox() {FormName = FormName, InputName = inputName, Value = value, IsVisible = isVisible} ; 
} else {
    input = CustomEditorInputs[GetCustomEditorInput(inputName, value)];
    input.inputDisplayName = CustomPropertyNames[input.InputName];
}
return input;

---

In an effort to fix my mistakes in the above code I re-wrote the code using the customEditorInputKey and ternary statements. I switched things around if it's not null it will retrieve the actual input object otherwise it will create new inputs (these should be added to the CustomEditorInputs list/collection/dictonary/{whatever you are going to use} )

string inputName = property.Name;
string value = Convert.ToString(property.GetValue(record, null));
bool isVisible = true;
var customEditorInputKey = GetCustomEditorInput(inputName, value) 

if (customEditorInputKey != null) { 
    input = CustomEditorInputs[customEditorInputKey];
    input.inputDisplayName = CustomPropertyNames[input.InputName];

} else {
    input = property.PropertyType.Name == "Boolean" 
        ? new ExpressFormsCheckBox() {FormName = FormName, InputName = inputName, Value = value, IsVisible = isVisible} 
        : new ExpressFormsTextBox() {FormName = FormName, InputName = inputName, Value = value, IsVisible = isVisible} ; 
}
return input;