-1

Есть у меня метод, который принимает на вход id прибора и значение, которое прибор показывает. В базе данных есть значение погрешности для этого прибора и диапазон погрешности. Метод мой вытаскивает прибор (вместе с ним все данные), и мне нужно также проверить показание этого прибора, вписывается в диапазон погрешности или нет, если вписывается, тогда я из метода возвращаю значение погрешности, а если не вписывается, тогда просто пустую строку. Вопрос, подскажите, можно ли как-то короче реализовать его, так вот я сделал. И правильно ли я проверил значение на принадлежность диапазону и вышел из метода.

 public static string GetInstrumentPogr(string id,string value)
        {

            //вытаскиваем из базы прибор по его id
            var intrument = GetInstrumentById(id);

            string insPorg = string.Empty; //финальный результат
            if(intrument != null)
            {
                bool rezult = true;
                double InnacuratyValue =Convert.ToDouble(intrument.Pogr);// значение погрешности
                double ll = CalcModule.getLowerLimit(intrument.DiapPogr);// значение нижней границы диапазона
                double ul = CalcModule.getUpperLimit(intrument.DiapPogr);// значение верхней границы диапазона
                double val = Convert.ToDouble(value);// значение(показание прибора), которое подается на вод метода.
                if (Math.Abs(val)>=ul && Math.Abs(val)>=ll)//попадает ли значение в диапазон
                {
                    rezult=false;
                }
                if (rezult==true)
                {
                    return insPorg = InnacuratyValue.ToString();//возвращаем значение погрешности для прибора
                }
                else
                {
                    return insPorg=string.Empty;// пустую строку
                }
            }

            return insPorg;//финальный результат
        }
10
  • 1
    Для полноты приведите, пожалуйста, класс Instrument, потому что из кода непонятно, какого типа instrument.DiapPogr 24 мар 2020 в 6:17
  • Это класс возвращает все данные по прибору, он работает правильно. 24 мар 2020 в 6:21
  • 2
    зачем Math.Abs? Отрицательные значения, если позволяет прибор, просто должны попадать между нижней и верхней границей?
    – 4per
    24 мар 2020 в 6:29
  • 1
    это должно храниться в intrument.DiapPogr
    – 4per
    24 мар 2020 в 6:33
  • 1
    а теперь проверьте, что будет с -60
    – 4per
    24 мар 2020 в 6:38

4 ответа 4

3

Можно упростить условие и избавиться от булевой переменной, она у вас все равно используется в одном лишь месте.

    public static string GetInstrumentPogr(string id,string value)
    {            
        var intrument = GetInstrumentById(id);
        string insPorg = string.Empty; //финальный результат
        if(intrument != null)
        {
            double InnacuratyValue =Convert.ToDouble(intrument.Pogr);
            double ll = CalcModule.getLowerLimit(intrument.DiapPogr);
            double ul = CalcModule.getUpperLimit(intrument.DiapPogr);
            double val = Convert.ToDouble(value);
            // Для принадлежности к диапазону 
            // значение должно быть больше минимального и меньше максимального
            if (Math.Abs(val)>ll && Math.Abs(val)<ul)
               insPorg = InnacuratyValue.ToString();                                 
        }
        return insPorg;
    }
0
1
public static string GetInstrumentPogr(string id, string value) {
    var intrument = GetInstrumentById(id);
    string insPorg = string.Empty;
    if(null != intrument) {
       double InnacuratyValue = Convert.ToDouble(intrument.Pogr),
              ll = CalcModule.getLowerLimit(intrument.DiapPogr),
              ul = CalcModule.getUpperLimit(intrument.DiapPogr),
              val = Convert.ToDouble(value);
       if (Math.Abs(val) < ul && Math.Abs(val) > ll) {
           insPorg = InnacuratyValue.ToString();
       }
    }
    return insPorg;
}
1

Предложу свое, более глобальное видение решения этой проблемы:

1) У вас метод занимается как минимум двумя вещами, а именно:

  • достает из базы инструмент
  • проверяет, попало ли значение в диапазон В хорошем коде этот метод нужно разбить как минимум на 2.

На вашем месте я бы вынес во внешний код var intrument = GetInstrumentById(id); из метода и передавал бы в метод сам инструмент:

public static string GetInstrumentPogr(Instrument instrument, string value)
{
    //...
}

2) У вас метод возвращает одинаковый результат (string.Empty), если инструмент не найден и если значение не попало в диапазон. Как вызывающий код поймет, что именно произошло (не найден инструмент или значение не попало в диапазон)?

3) if (Math.Abs(val)>=ul && Math.Abs(val)>=ll) - абсолютно неправильная проверка. Проверка на попадание в диапазон должна быть: if (val >= ll && val <= ul).

4) Сам класс Instrument у вас является дата-моделью. Я бы его так и назвал - InstrumentDataModel. А для инструмента сделал бы отдельный класс Instrument и поместил бы в него ваш метод GetPogr.

public class Instrument
{
    public double InnacuratyValue { get; }
    public double LowerLimit { get; }
    public double UpperLimit { get; }

    public Instrument(InstrumentDataModel dataModel)
    {
        // По-хорошему, эту проверку лучше делать до вызова конструктора. Для понимания происходящего поместил ее сюда
        if (dataModel == null)
        {
            throw new ArgumentNullException();
        }

        InnacuratyValue = Convert.ToDouble(dataModel.Pogr);

        // Предполагается, что в базе лежат правильные значения погрешности и LowerLimit <= UpperLimit
        LowerLimit = CalcModule.getLowerLimit(dataModel.DiapPogr);
        UpperLimit = CalcModule.getUpperLimit(dataModel.DiapPogr);
    }

    public string GetPogr(string value)
    {
        double val = Convert.ToDouble(value);// значение(показание прибора), которое подается на вод метода.

        var result = string.Empty;
        if (val >= LowerLimit && val <= UpperLimit)//попадает ли значение в диапазон
        {
            result = InnacuratyValue.ToString();
        }
        return result;
    }
}

Хоть и появился новый класс, код стал гораздо чище и понятнее. Теперь класс Instrument отвечает за валидацию значений и никто больше. Теперь пользоваться всем этим стало проще:

var instrumentDataModel = GetInstrumentById(id);
var instrument = new Instrument(instumentDataModel);
var pogr = instument.GetPogr(value);

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

0
0

Уберите из метода всё лишнее

public static bool TryGetInstrumentUncertainty (Instrument instrument, double value, out double uncertainty)
{
    uncertainty = Convert.ToDouble(instrument.Pogr);// значение погрешности
    double ll = CalcModule.getLowerLimit(instrument.DiapPogr);// значение нижней границы диапазона
    double ul = CalcModule.getUpperLimit(instrument.DiapPogr);// значение верхней границы диапазона
    return value<=ul && value>=ll;
}
2
  • Мне нужно возвращать именно стринговое значение или пусто или значение погрешности 24 мар 2020 в 6:55
  • 1
    Это не проблема, там где будете использовать этот метод, опишите правила отображения результата.
    – 4per
    24 мар 2020 в 6:57

Ваш ответ

By clicking “Отправить ответ”, you agree to our terms of service and acknowledge you have read our privacy policy.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.