HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Implementation of OOP for retrieving list of objects from database

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromimplementationobjectsdatabaseforretrievinglistoop

Problem

If I have a Person class that outlines the properties of a person and one of People that just contains a List like so:

public class Person
{
    public Person(){}
    public int personID {get;set;}
    public string name {get;set;}
    public string address {get;set;}
}

public class People
{
    public People(){}

    public List peoples {get;set;}

    public List getPeople()
    {
        List p = new List();
        //create database connection
        //open datareader

        Person onePerson = new Person();
        //write data to eahc property type and add data to list

        p.Add(onePerson);
        return p;
    }
}


To call the database method that gets the people from the database I could do this:

static void main(string[] args)
{
    People p = new People();
    p.getPeople();
}


Is this how I would do this in terms of OOP?

I'm just beginning to use these techniques in my coding as usually I would remove the getPeople() method from the People class and do this in main():

static void main(string[] args)
{
    List people = getPeople();
}

private static List getPeople()
{
    List p = new List();
    //create database connection
    //open datareader
    //load data into list

    Person onePerson = new Person();

    p.Add(onePerson);
    return p;
}

Solution

Very few things in life are free. C# default constructors are one of those, the compiler generates one automatically for you if your class doesn't explicitly have one! So unless you have another constructor (with parameters) and you also need a parameterless one, putting it in is just clutter.

Encapsulating database operations in a dedicated object (like you're doing with your People class), vs doing it all in a static method, is a step forward in the right direction.. but I don't see what the peoples property is trying to encapsulate.

Think of classes as abstractions - you have a Person abstraction which represents a single record in your database. Then you have a People abstraction which represents an object that can manipulate Person records. That's all nice and tidy and high-level - you don't want to mix this niceness with boilerplate-level SQL connections and queries. Doing that is writing code at the wrong level of abstraction.

So the next step forward would be to create another class that's responsible for dealing with these things - a type that would deal with setting up a connection, running a query and then disposing the connection and every other IDiposable along the way.

Once you've done that, you will (hopefully) realize you've just reinvented the wheel - and that your wheel is more than likely square. At this stage it's all good, the worst thing you could do is start with an Entity Data Model that does it all for you: you want to understand what's going on, if it feels automagic then you've skipped something.

So first (well, after you've written your square wheel!) take a look at Linq to SQL and see what a DataContext can do for you.

Context

StackExchange Code Review Q#33810, answer score: 5

Revisions (0)

No revisions yet.