CODEWITHSUNDEEP
c#design-patterns
AntiPatternMax
AntiPatternMax

Reputation: 31

Tips on refactoring unstructured repetitive code into reusable components

I have searched and searched and could not get any relevant answer. Hopefully someone can help.

I started writing classes as a sort of unit hold data and another util class to populate the class from the database. Well this might not seem to big of a problem for a small code base but looking forward and for me to grow to a competent developer with a solid grasp of object orientated design and development i need to grow this into a proper design

Lets start of with the initial class 

public class Foo
{
   public int FooID {get;set;}
   public string FooName {get;set;}
   public string FooDescription {get;set;}
}

public class NotRelatedToFoo
{
   public int SomeID {get;set;}
   public string SomeName {get;set;}
   public string SomeDescription {get;set;}
}


public class Util
{
   public Util(){} 

   public List<NotRelatedToFoo> GetNotRelatedToFoos(string SomeName)
   {
     .... DB Code to return Data from DB 

     List<NotRelatedToFoo> notfoos = new List<NotRelatedToFoo>()
     (select * from NotRelatedToFoo where SomeName like @SomeName)
     foreach var item in db
     {
       NotRelatedToFoo notfoo = new NotRelatedToFoo ();
       notfoo.SomeID = item.SomeID;
       notfoo.SomeName = item.SomeName
       notfoo.SomeDescription = item.SomeDescription 
       notfoos.Add(notfoo);
      }
      return notfoos ;
   }

}

My question is what is the best way to move forward from this design.

Edit At the moment the Util class is a big mess and i need advice how to extract and group the functions together follow a structured design

This is not going to work, or can it

public class Foo
{
   public int FooID {get;set;}
   public string FooName {get;set;}
   public string FooDescription {get;set;}

   public List<Foo> GetFoos(string FooName)
   {
     .... DB Code to return Data from DB 

     List<Foo> foos = new List<Foo>()
     (select * from foo where FooName like @FooName)
     foreach var item in db
     {
       Foo foo = new Foo();
       foo.FooID = item.FoodID;
       foo.FooName = item.FooName
       foo.FooDescription = item.FooDescription 
       foos.Add(foo);
      }
      return foos;
   }
}

Thanks

Upvotes: 2

Views: 839

Answers (3)

jason
jason

Reputation: 241651

My question is what is the best way to move forward from this design.

Classes named Util or Manager or what have you are generally a design smell. You should be able to describe what it is that the class is doing (it should have a single responsibility) and that should be the name of the class. If you have to use generic, vague, catch-all terms to describe what the class is doing then you are doing something wrong and a redesign is in order.

Your Util class is doing too much. It's getting data from a database for two distinct types. By designing your class in this way, you are setting yourself up to violate the open/closed principle (you will have to modify this class every time you add a new type to your model).

At a minimum, you need separate classes to get Foos and NotRelatedToFoos from a database. How about

abstract class Repository<T> {
     IEnumerable<T> GetAll();
}

abstract class SqlRepository<T> : Repository<T> {
    protected readonly SqlConnection connection;
    public SqlRepository(SqlConnection connection) {
        this.connection = connection;
    }
}

class FooRepository : SqlRepository<T> {

    public FooRepository(SqlConnection connection) : base(connection) { }

    public IEnumerable<T> GetAll() {
    }
}

etc. What's really sick is that you can even lift a lot of the repetitive code of hydrating an object from an IDataReader into a common class (hint: a first version would use reflection).

Beyond this, read about SOLID. And if you really want to dive it, try Patterns of Enterprise Application Architecture (Fowler) and Design Patterns (Gamma et al.)

Upvotes: 4

Michael
Michael

Reputation: 20049

Your Util class is responsible for many things (as Jason pointed out earlier).

It appears though that at the crux of it, Util is attempting to be a repository for your Foo and UnrelatedToFoo objects.

I would personally look at abstracting this out into some inheritance structures and introducing generics.

If you're going to be be creating a lot of these POCO objects that map to database tables in repetitive and convention based ways, consider an ORM such as NHibernate with FluentNHibernate mappings.

Upvotes: 0

myAces
myAces

Reputation: 699

the common way to access the database, is through data access objects. http://java.sun.com/blueprints/corej2eepatterns/Patterns/DataAccessObject.html

Don't know, if you have other classes for persisting, updating, or deleting data, but I usually do CRUD operations through a DAO.

Upvotes: 0

Related Questions