CODEWITHSUNDEEP
c#object
Scott Kramer
Scott Kramer

Reputation: 1731

c# object/design fundamentals

Is this a standard, good practice way of doing things? Basically return a list of itself? Should the actual fields (id, title, etc) be a separate class? (I've seen people call it DTO objects)

I'm starting a project & I want to try & get some of these fundamentals down--

Thanks!!

public class Calendar
{
    public int id { get; set; }
    public string title { get; set; }

    public List<calendar> GetAll()
    {
        var list = new List<calendar>();

        var db = new mssql2();
        db.set("s1");
        string sql = @"select * from [cal]";
        var dr = db.dr(sql);
        while (dr.Read())
        {
           var e = new calendar();
           e.id = (int)dr["id"];
           e.title = dr["title"].ToString();
           list.Add(e);
        }
        return list;
    }
}

Upvotes: 0

Views: 353

Answers (3)

ipavlic
ipavlic

Reputation: 4966

The general idea is for the classes to represent domain objects, and class members various properties of those domain objects. Class functions would represent what the objects can do.

In your case, it might be more fitting to remove the get_all() to a some class abstracting database operations. Calendar would have the functionalities of a calendar (getting/setting some dates, getting skip years, getting/setting some appointments); depending of what you want to accomplish with a calendar.

Object design

Upvotes: 2

Brandon
Brandon

Reputation: 70022

You seem to be mixing your Domain model with your Data Access layer.

Keep Calendar as it's own class, and maybe make another class called CalendarService or CalendarRepository that returns you a list of Calendar objects.

Here is an example:

public class Calendar
{
    public Calendar() { }
    public Calendar(int id, string title)
    {
       Id = id;
       Title = title;
    }
    public int Id { get; set; }
    public string Title { get; set; }
}

public class CalendarService
{
    public static List<Calendar> GetAll()
    {
        var list = new List<Calendar>();

        var db = new mssql2();
        db.set("s1");
        string sql = @"select * from [cal]";
        var dr = db.dr(sql);
        while (dr.Read())
        {
            // Use the constructor to create a new Calendar item
            list.Add(new Calendar((int)dr["id"], dr["title"].ToString()));
        }
        return list;
    }
}

Upvotes: 7

Sean H
Sean H

Reputation: 746

You're tightly coupling data access, and your "get_all" method isn't even using anything from the object of type calendar. If, as in this case, your method doesn't use any data from the instance of the class to which it belongs, then that method should either not be there, or should be a static method. My preference would be for the former -- have a class whose intent is to retrieve a calendar or calendars from the database. It is a more sensible organization of code, is more testable, can be more easily abstracted from the data layer, and it also makes your data object more portable.

Upvotes: 0

Related Questions