Как сделать этот код более удобочитаемым?

Ответ 1

Вы всегда можете использовать псевдоним для очень длинного типа RequestReportsCalculatingStoredProcedures:

using RRCSP = RequestReportsCalculatingStoredProcedures;

Примечание. Вам нужно будет использовать полное имя (включая пространство имен) в директиве using.

Ответ 2

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

switch (myMassiveVariable)
{
    case RequestReportStoredProcedureType.ReportPlanWithEffects:
        RunReportWithEffects(requestNo);
        break;
    case RequestReportStoredProcedureType.ReportPlanWithEffectsForFacts:
        RunReportWithFacts(requestNo);
        break;
}

Это помогает немного убирать вещи.

Ответ 3

Я твердо верю в описательные имена переменных, классов и методов. Я всегда выбирал бы ясность в краткости, однако одно легко реализуемое правило состоит в том, что если у вас есть часть ваших имен, которые всегда повторяются, вы можете избавиться от этого. Например, у вас есть:

RequestReportsCalculatingStoredProcedures

Это хорошо. Это объясняет, что это ясно. Однако у вас есть члены этого класса или объекта, которые также начинаются с этого имени плюс дифференцирование имен в конце. Например:

RequestReportStoredProcedureType

Это может быть сокращено до StoredProcedureType или даже, возможно, ProcedureType.

Я знаю, что многие программисты будут спорить о чем-то вроде RRCalcSP или некоторых других совершенно неясных соглашениях об именах, но я никогда не пожертвую ясностью именования, чтобы избежать обертки строк и т.д.

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

Кроме того, щедрое использование комментариев делает многое более понятным.

Ответ 4

Честно говоря, одна из самых больших проблем здесь - это просто длина имен переменных.

Очевидно, вы должны указать переменные/типы/и т.д. описательные имена. Но есть момент, когда он становится немного экстремальным. Один парень в моей работе известен тем, что дал имена методов, например:

DoSomethingVerySpecificHereIsOneOfItsSideEffectsAndHereIsAnother

В вашем случае я замечаю много избыточности. Например, у вас есть класс с именем RequestReportsCalculatingStoredProcedures, а затем внутри этого класса вы, похоже, имеете перечисление с именем RequestReportStoredProcedureType. Поскольку перечисление уже является вложенным типом внутри RequestReportsCalculatingStoredProcedures, возможно, вы могли бы просто назвать его Type?

В качестве альтернативы очень эффективным способом сокращения этих имен (по крайней мере, в этом файле) было бы использование простого объявления using:

using E = RequestReportsCalculatingStoredProcedures.RequestReportStoredProcedureType;

Затем посмотрите, что происходит с вашим кодом:

using RRCSP = RequestReportsCalculatingStoredProcedures;
using E = RRCSP.RequestReportStoredProcedureType;

// ...

// Note: RRCSP = RequestReportsCalculatingStoredProcedures, and
//       E = RRCSP.RequestReportStoredProcedureType

switch ((E)Enum.Parse(typeof(E), ihdType.Value))
    {
        //REF:This can (but should it?) be refactored through strategy pattern
        case E.ReportPlanWithEffects:
            grvEconomicCriteria.DataSource = RRCSP.ReportsDataParser(
                RRCSP.ReportPlanWithEffects(
                    requestNo,
                    RRCSP.GetAlgorithmNoByRequestNo(requestNo)
                )
            );
            break;
        case E.ReportPlanWithEffectsForFacts:
            DateTime factDate;
            try
            {
                factDate = Convert.ToDateTime(ihdDate.Value);
            }
            catch(FormatException)
            {
                grvEconomicCriteria.DataSource = RRCSP.ReportsDataParser(
                    RRCSP.ReportPlanWithEffectsForFacts(
                        requestNo,
                        RRCSP.GetAlgorithmNoByRequestNo(requestNo),
                        DateTime.MinValue
                    )
                );
                break;
            }
            grvEconomicCriteria.DataSource = RRCSP.ReportsDataParser(
                RRCSP.ReportPlanWithEffectsForFacts(
                    requestNo,
                    RRCSP.GetAlgorithmNoByRequestNo(requestNo),
                    factDate
                )
            );
            break;
        default:
            break;
    }

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

Ответ 5

Если у вас есть некоторый контроль над используемым кодом, я бы рекомендовал:

• Принесите enum RequestReportsCalculatingStoredProcedures.RequestReportStoredProcedureType для внешней области. Внутреннее имя уже достаточно многословно, что вам не требуется внешнее имя для уточнения.

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

• Предоставьте перегрузку RequestReportsCalculatingStoredProcedures.ReportPlanWithEffectsForFacts, которая не требует параметра алгоритма, так как вы, по-видимому, можете найти его в запросе. В любом случае. Это устраняет подробный вызов RequestReportsCalculatingStoredProcedures.GetAlgorithmNoByRequestNo, когда вам не нужно предоставлять нестандартный алгоритм.

Ответ 6

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

Класс RequestReportsCalculatingStoredProcedure Я бы переименовал в RequestReports. Часть StoredProcedure для меня, похоже, представляет собой деталь реализации, которая больше не является бизнесом. Я не уверен, что слово Calculating приводит к таблице, поэтому я также удалил это.

Перечисление RequestReportStoredProcedureType Я бы переименовал в ReportPlan, поскольку это, казалось, соответствовало контексту лучше (ReportType также является возможностью). Дополнительная словесность была удалена аналогично причинам, которые класс, который ее охватывает, был переименован. Я оставил его внутри класса, поскольку он, кажется, предоставляет некоторый контекст, и с сокращением названий это казалось хорошей идеей.

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

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

switch((RequestReports.ReportPlan)Enum.Parse(typeof(RequestReports.ReportPlan), ihdType.Value)) {
            //REF:This can (but should it?) be refactored through strategy pattern
            case RequestReports.ReportPlan.WithEffects:
                Object reportPlan = RequestReports.ReportPlanWithEffects(requestNo);
                grvEconomicCriteria.DataSource = RequestReports.ReportsDataParser(reportPlan);
                break;
            case RequestReports.ReportPlan.WithEffectsForFacts:
                DateTime factDate;
                try {
                    factDate = Convert.ToDateTime(ihdDate.Value);
                }
                catch(FormatException) {
                    Object reportPlan2 = RequestReports.ReportPlanWithEffectsForFacts(requestNo, DateTime.MinValue);
                    grvEconomicCriteria.DataSource = RequestReports.ReportsDataParser(reportPlan2);
                    break;
                }
                Object reportPlan3 = RequestReports.ReportPlanWithEffectsForFacts(requestNo, factDate);
                grvEconomicCriteria.DataSource = RequestReports.ReportsDataParser(reportPlan3);
                break;
            default:
                break;
            }

Ответ 7

Посмотрите с помощью команды.

Также я верю (не проверял), но вы можете сделать что-то похожее на:

RequestReportsCalculatingStoredProcedure myShortCut = RequestReportsCalculatingStoredProcedures.RequestReportStoredProcedureType;

Затем вместо ссылки на большие строки вы можете ссылаться на myShortCut.

Ответ 8

Вы можете применить шаблон стратегии по мере комментирования или использовать словарь с делегатами (Func < > или Action < > ), который на основе типа что-то делает. Вы можете сократить время try/catch DateTime с помощью метода DateTime.TryParse(). Помимо этого, что наиболее недопустимо из вашего кода, это длинные имена, поэтому вы можете захотеть поместить некоторые из них в переменную, прежде чем запускать свой код

надеюсь, что это поможет

Ответ 9

я защищал бы извлечение общих частей из утверждения дела

RequestReportsCalculatingStoredProcedures.RequestReportStoredProcedureType dependingOnType =(RequestReportsCalculatingStoredProcedures.RequestReportStoredProcedureType)Enum.Parse(typeof(RequestReportsCalculatingStoredProcedures.RequestReportStoredProcedureType), ihdType.Value);

var EssentialData = null;
var AlgorithmChosen = RequestReportsCalculatingStoredProcedures.GetAlgorithmNoByRequestNo(requestNo);

switch (dependingOnType)
    {
            //REF:This can (but should it?) be refactored through strategy pattern
        case RequestReportsCalculatingStoredProcedures.RequestReportStoredProcedureType.ReportPlanWithEffects:
           EssentialData  = RequestReportsCalculatingStoredProcedures.ReportPlanWithEffects(requestNo, AlgorithmChosen);
            break;
        case RequestReportsCalculatingStoredProcedures.RequestReportStoredProcedureType.ReportPlanWithEffectsForFacts:
            DateTime factDate = Datetime.MinValue;

            if (!DateTime.TryParse(ihdDate.Value, factDate)) {
                factDate = DateTime.MinValue;
            }

               EssentialData  = RequestReportsCalculatingStoredProcedures.ReportPlanWithEffectsForFacts(requestNo, AlgorithmChosen, factDate);

            break;
        default:
            break;
    }

grvEconomicCriteria.DataSource = RequestReportsCalculatingStoredProcedures.ReportsDataParser(essentialData);

Вы можете уточнить это событие, вычислив

RequestReportsCalculatingStoredProcedures.GetAlgorithmNoByRequestNo(requestNo)

только один раз

edit: вот так:)