views:

218

answers:

2

I am making a number of distinct controllers, one relating to each stored procedure in a database. These are only used to read data and making them available in JSON format for javascripts.

My code so far looks like this, and I'm wondering if I have missed any opportunities to re-use code, maybe make some help classes. I have way too little experience doing OOP, so any help and suggestions here would be really appreciated.

Here is my generalized code so far (tested and works);

using System;
using System.Configuration;
using System.Web.Mvc;
using System.Data;
using System.Text;
using System.Data.SqlClient;
using Prototype.Models;

namespace Prototype.Controllers
{
    public class NameOfStoredProcedureController : Controller
    {

        char[] lastComma = { ',' };

        String oldChar = "\"";
        String newChar = """;

        StringBuilder json = new StringBuilder();

        private String strCon = ConfigurationManager.ConnectionStrings["SomeConnectionString"].ConnectionString;
        private SqlConnection con;

        public StoredProcedureController()
        {
            con = new SqlConnection(strCon);
        }

        public string do_NameOfStoredProcedure(int parameter)
        {
            con.Open();

            using (SqlCommand cmd = new SqlCommand("NameOfStoredProcedure", con))
            {
                cmd.CommandType = CommandType.StoredProcedure;
                cmd.Parameters.AddWithValue("@parameter", parameter);


                using (SqlDataReader reader = cmd.ExecuteReader())
                {
                    while (reader.Read())
                    {
                        json.AppendFormat("[{0},\"{1}\"],", reader["column1"], reader["column2"]);
                    }
                }
                con.Close();
            }

            if (json.Length.ToString().Equals("0"))
            {
                return "[]";
            }

            else
            {
                return "[" + json.ToString().TrimEnd(lastComma) + "]";
            }
        }


        //http://host.com/NameOfStoredProcedure?parameter=value
        public ActionResult Index(int parameter)
        {
            return new ContentResult
            {
                ContentType = "application/json",
                Content = do_NameOfStoredProcedure(parameter)
            };
        }
    }
}
+5  A: 

I probably wouldn't directly access the database from the controller but would rather abstract this access. Not really a performance optimization but design improvement. So start by defining a model that will hold the result of the stored procedure:

public class MyModel
{
    public string Column1 { get; set; }
    public string Column2 { get; set; }
}

Then define a repository interface that will contain the different operations on this model:

public interface IRepository
{
    IEnumerable<MyModel> GetModel(int id);
}

Next implement the repository:

public class RepositorySql : IRepository
{
    public IEnumerable<MyModel> GetModel(int id)
    {
        using (var conn = new SqlConnection(ConfigurationManager.ConnectionStrings["SomeConnectionString"].ConnectionString))
        using (var cmd = conn.CreateCommand())
        {
            conn.Open();
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.CommandText = "NameOfStoredProcedure";
            cmd.Parameters.AddWithValue("@parameter", id);
            using (SqlDataReader reader = cmd.ExecuteReader())
            {
                while (reader.Read())
                {
                    yield return new MyModel
                    {
                        Column1 = reader["column1"].ToString(),
                        Column2 = reader["column2"].ToString()
                    };
                }
            }
        }
    }
}

Finally your controller will use the repository:

public class NameOfStoredProcedureController : Controller
{
    private readonly IRepository _repository;
    public NameOfStoredProcedureController(IRepository repository)
    {
        _repository = repository;
    }

    // Warning don't add this constructor. Use a DI framework instead.
    // This kind of constructors are called Poor Man DI (see http://www.lostechies.com/blogs/jimmy_bogard/archive/2009/07/03/how-not-to-do-dependency-injection-in-nerddinner.aspx)
    // for more info on why this is bad.
    public NameOfStoredProcedureController() : this(new RepositorySql())
    { }

    public ActionResult Index(int parameter)
    {
        var model = _repository.GetModel(parameter);
        // Use directly Json, no need to do the serialization manually
        return Json(model);
    }
}
Darin Dimitrov
@Darin - That looks like solid code, however the problem with reusing that is that each controller may rely on various types and amounts of input parameters, as well as various different amounts and types of output columns. Would you have a solution for that?Also I'd really like to know why you would not directly access the database from the controller, is it mostly because of bad object orientation?
cc0
If these columns and parameters have things in common define input and output models deriving from each other so that you have a nice object model. Don't think in terms of database columns: think in terms of entities and relation between those entities and write classes to represent them. As for why it is bad to directly access the database from the controller the answer is that your controller is tied to this particular implementation and is impossible to unit test in isolation.
Darin Dimitrov
That is awesome. Thank you for taking the time to help me with this. Excellent.
cc0
+2  A: 

Quite often I do things manually myself, but have you taken a look at JsonResult ( Example )?

And also JavaScriptSerializer?

And also JSON.Net?

kervin
I have, but I found it easiest (although yes, a bit ugly code) to just use a stringbuilder when I needed to customize the json format. These are still relevant to the question though, thank you for pointing it out. Hopefully it can be useful for others reading this.
cc0
I understand, I use the stringbuilder approach too sometimes.
kervin