Новичок в ООП; это хороший дизайн класса?


У меня есть несколько разных классов в моем приложении, но этот конкретный класс является одним мне интересно про. Пожалуйста, ознакомьтесь с передовой практики и возможных проблем.

using ExactEarth.Utilities.Jaguar.Properties;
using System;
using System.Data;
using System.Data.SqlClient;
using System.IO;

public class Downlink
{
    public string PassId { get; set; }
    public string DataId { get; set; }
    public string NoradId { get; set; }
    public string Spacecraft { get; set; }
    public string GroundStation { get; set; }
    public string Start { get; set; }
    public string End { get; set; }
    public string Status { get; set; }

    private static string connectionString;
    private static SqlConnection connection;

    public Downlink(string passId)
    {
        using (StreamReader reader = new StreamReader(AppDomain.CurrentDomain.BaseDirectory + Resources.connectionStringPath))
        {
            connectionString = reader.ReadLine();
        }

        using (connection = new SqlConnection(connectionString))
        {
            connection.Open();
            this.PassId = passId;
            this.DataId = GetDataId();
            this.NoradId = GetSatelliteId();
            this.Spacecraft = GetSpacecraft();
            this.GroundStation = GetGroundStation();
            this.Start = GetStartTime();
            this.End = GetEndTime();
            this.Status = GetPassStatus();
        }
    }

    public string GetDataId()
    {
        string dataId = null;

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@passId", SqlDbType.NVarChar).Value = this.PassId;
            command.CommandText = Resources.passDataQuery;
            command.Connection = connection;
            SqlDataReader reader = command.ExecuteReader();
            reader.Read();
            dataId = reader["gs_pass_data_id"].ToString();
            reader.Close();
        }

        return dataId;
    }

    public string GetSatelliteId()
    {
        string satelliteId = null;

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@dataId", SqlDbType.NVarChar).Value = this.DataId;
            command.CommandText = Resources.ausInPassQuery;
            command.Connection = connection;
            SqlDataReader reader = command.ExecuteReader();
            if (reader.HasRows)
            {
                reader.Read();
                satelliteId = reader["acq_id"].ToString().Substring(0, 5);
            }

            return satelliteId;
        }
    }

    public string GetSpacecraft()
    {
        string path = AppDomain.CurrentDomain.BaseDirectory;
        path += Resources.spacecraftPath;

        using (StreamReader reader = new StreamReader(path))
        {
            string input = null;
            while ((input = reader.ReadLine()) != null)
            {
                string satelliteId = input.Split(',')[0];

                if (this.NoradId == satelliteId)
                    return input.Split(',')[1];
            }
        }

        // Spacecraft not known
        return Resources.unknownSpacecraft;
    }

    public string GetGroundStation()
    {
        string station = null;

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@dataId", SqlDbType.NVarChar).Value = this.DataId;
            command.CommandText = Resources.groundStationQuery;
            command.Connection = connection;
            SqlDataReader reader = command.ExecuteReader();
            if (reader.HasRows)
            {
                reader.Read();
                station = reader["ground_station_name"].ToString();
            }

            return station;
        }
    }

    public string GetStartTime()
    {
        string start = null;

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@dataId", SqlDbType.NVarChar).Value = this.DataId;
            command.CommandText = Resources.passStartQuery;
            command.Connection = connection;
            SqlDataReader reader = command.ExecuteReader();
            reader.Read();
            start = Convert.ToDateTime(reader["downlink_start_time"]).ToString("yyyy-MM-dd HH':'mm':'ss");
            reader.Close();
        }

        return start;
    }

    public string GetEndTime()
    {
        string end = null;

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@dataId", SqlDbType.NVarChar).Value = this.DataId;
            command.CommandText = Resources.passEndQuery;
            command.Connection = connection;
            SqlDataReader reader = command.ExecuteReader();
            reader.Read();
            end = Convert.ToDateTime(reader["downlink_end_time"]).ToString("yyyy-MM-dd HH':'mm':'ss");
            reader.Close();
        }

        return end;
    }

    public string GetPassStatus()
    {
        string status = null;

        if (!JaguarUtilities.IsSatelliteTracked(this.Spacecraft))
            status = Resources.statusNotTracked;

        else
        {
            using (SqlCommand command = new SqlCommand())
            {
                command.Parameters.Add("@passId", SqlDbType.NVarChar).Value = this.PassId;
                command.CommandText = Resources.sourceFileQuery;
                command.Connection = connection;
                SqlDataReader reader = command.ExecuteReader();

                if (reader.HasRows)
                {
                    reader.Close();

                    using (SqlCommand newCommand = new SqlCommand())
                    {
                        newCommand.Parameters.Add("@dataId", SqlDbType.NVarChar).Value = this.DataId;
                        newCommand.CommandText = Resources.passStatusQuery;
                        newCommand.Connection = connection;
                        reader = newCommand.ExecuteReader();
                        status = Resources.statusProcessing;
                        if (reader.HasRows)
                            status = Resources.statusProcessed;
                    }
                }

                else
                {
                    status = Resources.statusScheduled;

                    if (Convert.ToDateTime(this.End) < DateTime.UtcNow)
                    {
                        TimeSpan span = DateTime.UtcNow.Subtract(Convert.ToDateTime(this.End));
                        TimeSpan failTime = JaguarUtilities.GetFailTime(this.Spacecraft, this.GroundStation);

                        if (TimeSpan.Compare(span, failTime) <= 0)
                            status = Resources.statusUpdate + (Convert.ToDateTime(this.End) + failTime).ToString("HH':'mm");

                        else
                            status = Resources.statusWaiting;
                    }
                }
            }
        }

        return status;
    }

    public bool IsEscalated()
    {
        bool escalated;

        using (connection)
        {
            if (IsLastSpacePassFail())
                escalated = true;

            else if (IsNextSpacePassFail())
                escalated = true;

            else if (IsLastGroundPassFail())
                escalated = true;

            else if (IsNextGroundPassFail())
                escalated = true;

            else
                escalated = false;
        }

        return escalated;
    }

    private bool IsProcessingTooLong()
    {
        string path = AppDomain.CurrentDomain.BaseDirectory + Resources.processingTimePath;
        using (StreamReader reader = new StreamReader(path))
        {
            string time = reader.ReadLine();
            TimeSpan span = DateTime.UtcNow.Subtract(Convert.ToDateTime(this.End));
            TimeSpan failTime = JaguarUtilities.GetFailTime(this.Spacecraft, this.GroundStation);
            TimeSpan fail = new TimeSpan(0, int.Parse(time), 0);

            if (span > (fail + failTime))
                return true;

            else
                return false;
        }
    }

    private bool IsLastSpacePassFail()
    {
        string passId = null;
        connection.ConnectionString = connectionString;
        connection.Open();

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@satelliteId", SqlDbType.NVarChar).Value = this.NoradId;
            command.Parameters.Add("@downlinkTime", SqlDbType.NVarChar).Value = this.Start;
            command.CommandText = Resources.lastSpaceDataQuery;
            command.Connection = connection;

            SqlDataReader reader = command.ExecuteReader();
            reader.Read();
            passId = reader["gs_pass_id"].ToString();
        }

        Downlink lastDownlink = new Downlink(passId);
        if (lastDownlink.Status == Resources.statusWaiting)
            return true;

        else if (lastDownlink.Status == Resources.statusProcessing)
        {
            if (lastDownlink.IsProcessingTooLong())
                return true;
            else
                return false;
        }

        else
            return false;
    }

    private bool IsNextSpacePassFail()
    {
        string passId = null;
        connection.ConnectionString = connectionString;
        connection.Open();

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@satelliteId", SqlDbType.NVarChar).Value = this.NoradId;
            command.Parameters.Add("@downlinkTime", SqlDbType.NVarChar).Value = this.End;
            command.CommandText = Resources.nextSpaceDataQuery;
            command.Connection = connection;

            SqlDataReader reader = command.ExecuteReader();
            reader.Read();
            passId = reader["gs_pass_id"].ToString();
        }

        Downlink lastDownlink = new Downlink(passId);
        if (lastDownlink.Status == Resources.statusWaiting)
            return true;

        else if (lastDownlink.Status == Resources.statusProcessing)
        {
            if (lastDownlink.IsProcessingTooLong())
                return true;
            else
                return false;
        }

        else
            return false;
    }

    private bool IsLastGroundPassFail()
    {
        string passId = null;
        connection.ConnectionString = connectionString;
        connection.Open();

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@groundId", SqlDbType.NVarChar).Value = this.GroundStation;
            command.Parameters.Add("@downlinkTime", SqlDbType.NVarChar).Value = this.Start;
            command.CommandText = Resources.lastGroundDataQuery;
            command.Connection = connection;

            SqlDataReader reader = command.ExecuteReader();
            reader.Read();
            passId = reader["gs_pass_id"].ToString();
        }

        Downlink lastDownlink = new Downlink(passId);
        if (lastDownlink.Status == Resources.statusWaiting)
            return true;

        else if (lastDownlink.Status == Resources.statusProcessing)
        {
            if (lastDownlink.IsProcessingTooLong())
                return true;
            else
                return false;
        }

        else
            return false;
    }

    private bool IsNextGroundPassFail()
    {
        string passId = null;
        connection.ConnectionString = connectionString;
        connection.Open();

        using (SqlCommand command = new SqlCommand())
        {
            command.Parameters.Add("@groundId", SqlDbType.NVarChar).Value = this.GroundStation;
            command.Parameters.Add("@downlinkTime", SqlDbType.NVarChar).Value = this.End;
            command.CommandText = Resources.nextGroundDataQuery;
            command.Connection = connection;

            SqlDataReader reader = command.ExecuteReader();
            reader.Read();
            passId = reader["gs_pass_id"].ToString();
        }

        Downlink lastDownlink = new Downlink(passId);
        if (lastDownlink.Status == Resources.statusWaiting)
            return true;

        else if (lastDownlink.Status == Resources.statusProcessing)
        {
            if (lastDownlink.IsProcessingTooLong())
                return true;
            else
                return false;
        }

        else
            return false;
    }

    public int GetNumberAusInPass(string query)
    {
        int planned = 0;
        using (connection)
        {
            connection.ConnectionString = connectionString;
            connection.Open();

            using (SqlCommand command = new SqlCommand())
            {
                command.Parameters.Add("@dataId", SqlDbType.NVarChar).Value = this.DataId;
                command.CommandText = query;
                command.Connection = connection;
                SqlDataReader reader = command.ExecuteReader();
                while (reader.Read())
                {
                    ++planned;
                }

                reader.Close();
            }

            return planned;
        }
    }

    public int GetNumberAusMatched()
    {
        int matched = 0;

        using (connection)
        {
            connection.ConnectionString = connectionString;
            connection.Open();

            using (SqlCommand command = new SqlCommand())
            {
                command.Parameters.Add("@dataId", SqlDbType.NVarChar).Value = this.DataId;
                command.CommandText = Resources.producedAusQuery;
                command.Connection = connection;
                SqlDataReader reader = command.ExecuteReader();
                while (reader.Read())
                {
                    long num;
                    if (long.TryParse(reader["acquisition_id"].ToString().Split('_')[1], out num))
                        ++matched;
                }

                reader.Close();
            }

            return matched;
        }
    }
}


5537
6
задан 25 августа 2011 в 04:08 Источник Поделиться
Комментарии
3 ответа

Во-первых, я бы предложил что-нибудь почитать? Твердые ООП принципы проектирования

Сейчас, на ваш код. Две вещи выпрыгнет на меня. Ваш класс очень легко злоупотребить, и вы пытаетесь сделать слишком много вещей с одного класса.

1 Легкий для использования

Вы выставляете ряд общих свойств, которые можно заполнять из конструктора. Кроме того, вы подвергаете фактические методы, используемые для заполнения этих полей из базы данных.

Почему это так важно? Это означает, что я могу построить свой класс, запросите свойство DataId, тогда звоните GetDataId и получите различные значения.

Кроме того, устанавливая различные свойства, не имеет никакого эффекта. По крайней мере, вы должны объявить эти свойства с собственным набором функций.

общественного строка DataId { получить; личное набор; }

2 Вы дали этот класс слишком много обязанностей.

Мне кажется, что вы хотите сделать две вещи с этого класса. Значения доступ из базы данных и передавать их в другие части вашего приложения.

Разделить на два класса. Написать один класс, который умеет подключаться и извлекать значения из базы данных и другого класса, чья единственная задача состоит в перемещении этих ценностей. Это второй класс, который я описываю, обычно упоминается как объект передачи данных, или DTO.

Класс базы данных создание и заполнение ДТО по запросу для вас. Переезд в этот шаблон решает проблему для вас тоже.

Вы также, кажется, есть некоторые бизнес-логики, встроенной в этот класс. Такие функции, как IsNextGroundPassFail представляется принятие решений на основе данных в базе данных. Решения, основанные на данных и должны быть в отдельном классе от код для чтения и записи базы данных.

Резюме:

Я думаю, вам нужно как минимум три класса.


  1. Взаимодействие С Базой Данных Объектов

  2. Объект Передачи Данных

  3. Бизнес-Логика Объекта

13
ответ дан 25 августа 2011 в 05:08 Источник Поделиться

Нет, его не....

во-первых, весь код базы данных смешивается. Это лучше быть сделано, используя что-то вроде NHibernate на / и EntityFramework или некоторые подобные вещи. ( много различных мнений о техник, чтобы использовать там! )

но ты эндап с чем-то подобным

DataLink link = Repository.GetDatalinkByPassId("mypassid");

что бы уйти и создать объект данных из базы данных и заполнения всех полей и, если применимо, нагрузки другие объекты, линии передачи данных ссылки.

Рядом есть много спутников концепций / станций / и т. д., которые представляются в виде строк. Там могут быть предметы, которые вы хотите извлечь.

Но вы могли бы также хотеть абстрактных вещах немного, что его не между наземной станцией и сидел, но между узлами? или, возможно, некоторые другие абстрактные понятия. Я не знаю достаточно о домене предложить реальных абстракций, но ключ в любой системе ОО-найти абстракций.

Если у вас есть время, положите его в объект datetime, а не строку

Мое подозрение, что вы используете этот класс в основном для индикации. Это было бы лучше заниматься через "модель представления" модели представления бы принять что-то вроде канала связи объекта и преобразовывать вещи, как время, в форме, что для отображения ( было бы за что отвечает строка формата используется для преобразования времени, чтобы что-то отображаемые например, и может обрабатывать различные форматы времени, в зависимости от того, как раз самый разумный способ г, или глупо способ м д г ;-) )

1
ответ дан 25 августа 2011 в 10:08 Источник Поделиться

Согласна с предыдущими комментариями по поводу разделения класса и запутанный интерфейс.

Кроме того, я предлагаю вам начать использовать разработку через тестирование, это делает его легче, например:


  • выяснить, какой должна быть общественность против того, что должно быть частным (если другие классы не использовать "читать/получить" методы и нужны только свойства, зачем подвергать детали реализации),

  • какими должны быть статические (в идеале, ничего не было, иначе вы потеряете легкость тестирования в изоляции, и простота расширения на более поздних этапах разработки),

  • разделение обязанностей (доступ к базе данных и объекты данных и бизнес-логики).

0
ответ дан 3 сентября 2011 в 07:09 Источник Поделиться