3
\$\begingroup\$

I have several controls that are bound from database tables. Putting my OOP thinking cap on I figured since all of the controls will have a SqlCommand name and an associated SqlDataReader object I decided to make the following interface to program to.

public interface IBoundControl
    {
         string GetCommandName();
         void ReadyReader(SqlDataReader rdr);
    }

For illustration's sake, consider the following to classes which implement IBoundControl interface

public class Hospital : IBoundControl
    {

        public int HospitalId { get; set; }
        public string HospitalName { get; set; }

        public string GetCommandName()
        {
            return "spGetHospitals";
        }
        public void ReadyReader(SqlDataReader rdr)
        {
            this.HospitalId = (int)rdr["HospitalId"];
            this.HospitalName = (string)rdr["HospitalName"];
        }

    }

public class Race : IBoundControl
    {
        public int RaceId { get; set; }
        public string RaceDescription { get; set; }
        public string GetCommandName()
        {
            return "spGetRaces";
        }

        public void ReadyReader(SqlDataReader rdr)
        {
            this.RaceId = (int)rdr["RaceId"];
            this.RaceDescription = (string)rdr["RaceDescription"];
        }
    }

The controls are bound to a webform by making a call to the DataAccess class's BindControl method shown below:

 public class DataAccess
    {
        private readonly string cs = ConfigurationManager.ConnectionStrings["dbcs"].ConnectionString;

        //takes in a parameter that will be bound to a web form
        public List<IBoundControl> BindControl(IBoundControl i)
        {
            //creates a list of objects which will be bound to drop down lists
            List<IBoundControl> controlList = new List<IBoundControl>();
            using (var con = new SqlConnection(cs))
            {
                //grabs the name of the sproc for whatever type 'i' is
                using (var cmd = new SqlCommand(i.GetCommandName(), con))
                {
                    con.Open();
                    cmd.CommandType = CommandType.StoredProcedure;
                    SqlDataReader rdr = cmd.ExecuteReader();
                    while (rdr.Read())
                    {
                        //the factory creates an object of whatever type 'i' is
                        ControlFactory factory = new ControlFactory();
                        IBoundControl ibound = factory.CreateControl(i);
                        ibound.ReadyReader(rdr);
                        controlList.Add(ibound);
                    }
                }
                return controlList;
            }
        }

        public class ControlFactory
        {
            public IBoundControl CreateControl(IBoundControl control)
            {
                if (control is Hospital)
                    return new Hospital();
                else if (control is Race)
                    return new Race();
                else
                    return null;
            }
        }
    }

To my surprise, this actually worked - the first time, too! I was hoping to get some comments on how this might be improved upon, or if there's any general techniques that while this example might be too rudimentary to show, is bad practice down the road.

EDIT: When I first wrote this, the only controls that I had in mind were drop down controls(which is why both classes have two propeties, an ID and a description).

\$\endgroup\$
2
\$\begingroup\$

The IBoundControl is good. It isolates the specific data calls. It's refreshing to see another approach than falling back to the repository pattern.

IBoundControl implementation is on a class that also doubles as the DTO. This is breaking the SRP. One way to solve this is to create class that's only job is for lookup values. Call it Lookup. It has two fields: Id and Name.

The new interface looks like this:

public interface IBoundControl
    {
         string GetCommandName();
         List<Lookup> ReadyReader(IDataReader rdr);
    }

Now the IBoundControl implementation will only contain data layer specific code and will return a normalized resultset of lookup values. Also notice that IBoundControl.ReadyReader is taking a dependency on IDataReader instead of implementation specific SqlDataReader.

\$\endgroup\$
  • \$\begingroup\$ Can you elaborate on how exactly this would work? I can kind of envision what it would look like if it were Lookup was an interface and a called to ILookup.Id would return HospitalId, or RaceId, but I can't envision the method you're talking about in my head. \$\endgroup\$ – wootscootinboogie Aug 20 '13 at 13:06
1
\$\begingroup\$

I would:

  1. Replace string GetCommandName(); with string CommandName { get; }
  2. Implement abstract class:

    abstract class BoundControlBase : IBoundControl
    {
        public abstract void ReadyReader(SqlDataReader rdr);
    
        public string CommandName { get; private set; }
    
        protected BoundControlBase(string commandName)
        {
            CommandName = commandName;
        }
    }
    
  3. if keys are always correspond to property names, i would use reflection :) :

    //in base class
    protected void ReadProperty(SqlDataReader rdr, Expression<Func<object>> expression)
    {
        var lambda = (LambdaExpression)expression;
        var memberExpression = (MemberExpression)lambda.Body;
        var propertyInfo = (PropertyInfo)memberExpression.Member;
        propertyInfo.SetValue(this, rdr[propertyInfo.Name], null);
    }
    
    //usage
    ReadProperty(rdr, () => RaceId);
    

    if not: then i agree with Chuck - you need another layer of abstraction to encapsulate lookup logic.

  4. replace return null in CreateControl method with throw new NotSupportedException("blah-blah");

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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