Отрефакторите метод. Очень хочется разобрать свои ошибки

@Alone11 · Регистрация: 14.10.2017

Author24 — интернет-сервис помощи студентам

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

Суть метода: найти индекс скобок внутри текста, в нутри которых нет ничего или только другие пустые скобки максимальной вложенности.
Пример:
( ( ( ( ) А ) ) )
0 1 2 3 4 5 6 7 8

найдет индексы: 0 1 3 4 7 8

Конкретные вопросы:
п1. Как корректно заменить использование конструкции if(){} как проходного значения.
п2. Как желательно заменить использование if else внутри другого if
п3. По поводу буферной переменной, как можно было бы без нее.

C#

        private List<int> FindEmptyBrackets(string text) 
        {
            List<int> indexBracketOpen = new List<int>();  // п3. 
            List<int> indexBracketOpenFinal = new List<int>(); 
            List<int> indexBracketClose = new List<int>();
 
            List<int> indexBracketFull = new List<int>();
 
            bool bracketIsEpmty = true;
 
            for (int i = 0; i < text.Length; i++)
            {
                if (text[i].Equals('(') || text[i].Equals(' '))  
                {
                    if (text[i].Equals('(')) { indexBracketOpen.Add(i); bracketIsEpmty = true; } // п1.
                }
                else if (text[i].Equals(')') & indexBracketOpen.Count > 0)
                {
                    if (bracketIsEpmty)     // п2.
                    {
                        indexBracketClose.Add(i);
                        indexBracketOpenFinal.Add(indexBracketOpen.Last());
                        indexBracketOpen.RemoveAt(indexBracketOpen.Count - 1);
                        bracketIsEpmty = true;
                    }
                    else
                    {
                        indexBracketOpen.RemoveAt(indexBracketOpen.Count - 1); 
                    }
                }
                else
                {
                    bracketIsEpmty = false;
                }
            }
 
            foreach (var e in indexBracketOpenFinal)
            {
                indexBracketFull.Add(e);
            }
 
            foreach (var e in indexBracketClose)
            {
                indexBracketFull.Add(e);
            }
 
            return indexBracketFull;
        }

@SuprSonic · 18.01.2019, 19:50

Отказаться от List. Скорость повысится

@Lexeq · 19.01.2019, 00:17

Alone11, я бы так сделал
deleted

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

@o6938497 · 19.01.2019, 00:19

Не по теме:

Почему бы через регулярки не распарсить?

@Lexeq · 19.01.2019, 00:30

Alone11, во

C#

        private static List<int> FindEmptyBrackets(string text)
        {
            Stack<int> openedBracketIndices = new Stack<int>();
            var emptyBracketIndices = new List<int>();
 
            bool hasContent = false;
            int counter = -1;
            foreach (var ch in text)
            {
                if (ch == ' ')
                    continue;
 
                counter++;
                if (ch == '(')
                {
                    openedBracketIndices.Push(counter);
                }
                else if (ch == ')')
                {
                    var index = openedBracketIndices.Pop();
                    if (!hasContent)
                    {
                        emptyBracketIndices.Add(index);
                        emptyBracketIndices.Add(counter);
                    }
                    hasContent = false;
                }
                else
                {
                    hasContent = true;
                }
            }
            emptyBracketIndices.Sort();
            return emptyBracketIndices.ToList();
        }

@Alone11 · 19.01.2019, 14:31 **[ТС]**

Хотелось бы увидеть как лаконичнее через регулярки написать.

Добавлено через 15 минут
Lexeq, Спасибо, вышло меньше лапши, про Stack забыл, редко использовал.
Но по существу тут у меня вопрос был скорее как преодалеть if с внутренними if'ами, o6938497, посоветовал через регулярки, оно и понятно, тоже так думал, так как поиск по массиву и все такое, бы ло бы интересно посмотреть на корректный запрос. Но всегда ли можно обойтись регулярками, у меня тут скорее вопрос как в случае наобходимости ввода кучи разных условий не писать if внутри if, знакомый сказал что это очень плохой тон. Вот я по случаю и хотел быузнать от них как от них отказываться.(Как вариант выводить все в подметоды, но не всегда же получиться выделть целый метод под какуюто пустяковую операцию)

@КОП · 19.01.2019, 15:36

Сообщение от Alone11

Суть метода: найти индекс скобок внутри текста, в нутри которых нет ничего или только другие пустые скобки максимальной вложенности.
Пример:
( ( ( ( ) А ) ) )
0 1 2 3 4 5 6 7 8
найдет индексы: 0 1 3 4 7 8

начнем с постановки задачи, ваш код выводит 3 4. Но никак не 0 1 3 4 7 8
далее, на (((()A))((()))) вообще выведет кашу. полагаю правильный ответе - два индекса, 8 13

исходя из условия максимальной вложенности 3 4 будут проигнорированы

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

Сообщение от Alone11

Но всегда ли можно обойтись регулярками

скорее наоборот, всегда можно без них

Добавлено через 5 минут

Не по теме:

Сообщение от o6938497

Почему бы через регулярки не распарсить?

а сможете такую регулярку написать? :)
а объяснить начинающему?

@Alone11 · 19.01.2019, 16:37 **[ТС]**

КОП, Спасибо, да нашел у себя ошибку, почему-то скопировал без строчки кода одной. Теперь добавил.
Теперь при (((()A))((()))), все корректно отображается, но так как кол-во ( и ) не равное то первая скобка отмечается как корректная, для поиска незакрытых скобок у меня другой метод, здесь не преведен, так как суть поста именно в том чтоб услышать мнение опытных как делать корректный код с минимумом if else, тоесть, есть ли приемы которые желательны вместо if else внутри if else.

C#

 private List<int> FindEmptyBrackets(string text) 
        {
            List<int> indexBracketOpen = new List<int>();  // п3. 
            List<int> indexBracketOpenFinal = new List<int>(); 
            List<int> indexBracketClose = new List<int>();
 
            List<int> indexBracketFull = new List<int>();
 
            bool bracketIsEpmty = true;
 
            for (int i = 0; i < text.Length; i++)
            {
                if (text[i].Equals('(') || text[i].Equals(' '))  
                {
                    if (text[i].Equals('(')) { indexBracketOpen.Add(i); bracketIsEpmty = true; } // п1.
                }
                else if (text[i].Equals(')') & indexBracketOpen.Count > 0)
                {
                    if (bracketIsEpmty)     // п2.
                    {
                        indexBracketClose.Add(i);
                        indexBracketOpenFinal.Add(indexBracketOpen.Last());
                        indexBracketOpen.RemoveAt(indexBracketOpen.Count - 1);
                        bracketIsEpmty = true;
                    }
                    else
                    {
                        indexBracketOpen.RemoveAt(indexBracketOpen.Count - 1); 
                        bracketIsEpmty = true; // забыл добавить.
                    }
                }
                else
                {
                    bracketIsEpmty = false;
                }
            }
 
            foreach (var e in indexBracketOpenFinal)
            {
                indexBracketFull.Add(e);
            }
 
            foreach (var e in indexBracketClose)
            {
                indexBracketFull.Add(e);
            }
 
            return indexBracketFull;
        }

Добавлено через 10 минут
КОП,

Сообщение от КОП

Сообщение от Alone11
Но всегда ли можно обойтись регулярками
скорее наоборот, всегда можно без них

А вот тут, можно по подробнее, ну тоесть, в теме в целом, мне привели 2 варианта развития событий:

1. - Lexeq, Написал более оптимизированный код, но в целом оставил if else внутри if else так же как у меня.
2. - o6938497, предложил регулярки, пример не привел, но как вариант норм.

Вы сказали что всегда можно без них, тоесть лучше if else вместо регулярок, если выбор есть? Еще как нибуть можно, повторюсь, мне рекомендовали выводиль любые if else в отдельные методы, верно ли это?

@КОП · 19.01.2019, 17:35

Сообщение от Alone11

(((()A))((()))), все корректно отображается, но так как кол-во ( и ) не равное то первая скобка отмечается как корректная

вы точно считали?

Сообщение от Alone11

как делать корректный код

сформулировать условие, которое себе не противоречит!

Добавлено через 54 минуты

Сообщение от Alone11

Lexeq, Написал более оптимизированный код

это вы так подумали, на самом деле нет

Сообщение от Alone11

как вариант норм.

это вы так подумали, на самом деле мы еще не определились с задачей

Сообщение от Alone11

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

и в третий раз ваши домыслы. можно не значит лучше, но и не значит хуже.

Сообщение от Alone11

мне рекомендовали выводиль любые if else в отдельные методы, верно ли это?

нет. только когда это целесообразно.
Основныепризнаки того что это ВОЗМОЖНО нужно сделать, это когда в ифе много строк кода и когда этот код переиспользуется

@Alone11 · 19.01.2019, 19:37 **[ТС]**

Сообщение от КОП

Сообщение от Alone11
(((()A))((()))), все корректно отображается, но так как кол-во ( и ) не равное то первая скобка отмечается как корректная
вы точно считали?

Блин, что то я не внимательный слишком. Когда себе код копировал, скопировал не до конца. Но в любом случае, теперь код работает та как надо.

Добавлено через 7 минут

Сообщение от КОП

Сообщение от Alone11
как делать корректный код
сформулировать условие, которое себе не противоречит!

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

@КОП · 19.01.2019, 20:25

Сообщение от Alone11

Не совсем понял в чем противоречие

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

Сообщение от Alone11

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

Для ()(()) код выдаст все индексы.
Согласно словесному условию, нужны скобки максимальной вложенности, то есть только эти ()[()], так как у них вложенность 2, а у других 1 - то есть просто пустые скобки.

вторая часть ((a()))
ваш код выдаст обе крайние скобки [[a()]]
однако ([a()]) содержит "а", а внешние скобки [(a())] содержат в себе другие непустые скобки

@SuprSonic 7 / 4 / 3 Регистрация: 07.08.2016 Сообщений: 65
	18.01.2019, 19:50	2
	Отказаться от List. Скорость повысится 0

@Lexeq 1150 / 742 / 483 Регистрация: 21.01.2014 Сообщений: 1,903
	19.01.2019, 00:17	3
	Alone11, я бы так сделал deleted Добавлено через 14 минут ps. нашел багу, и не увидел, что там пробелы могут идти, ща скину новый код. 0

@o6938497
	19.01.2019, 00:19 #4
	Не по теме: Почему бы через регулярки не распарсить? 0

@Alone11 4 / 4 / 0 Регистрация: 14.10.2017 Сообщений: 85
	19.01.2019, 14:31 [ТС]	6
	Хотелось бы увидеть как лаконичнее через регулярки написать. Добавлено через 15 минут Lexeq, Спасибо, вышло меньше лапши, про Stack забыл, редко использовал. Но по существу тут у меня вопрос был скорее как преодалеть if с внутренними if'ами, o6938497, посоветовал через регулярки, оно и понятно, тоже так думал, так как поиск по массиву и все такое, бы ло бы интересно посмотреть на корректный запрос. Но всегда ли можно обойтись регулярками, у меня тут скорее вопрос как в случае наобходимости ввода кучи разных условий не писать if внутри if, знакомый сказал что это очень плохой тон. Вот я по случаю и хотел быузнать от них как от них отказываться.(Как вариант выводить все в подметоды, но не всегда же получиться выделть целый метод под какуюто пустяковую операцию) 0

@КОП 1123 / 794 / 219 Регистрация: 15.08.2010 Сообщений: 2,185
	19.01.2019, 15:36	7
	Сообщение от Alone11 Суть метода: найти индекс скобок внутри текста, в нутри которых нет ничего или только другие пустые скобки максимальной вложенности. Пример: ( ( ( ( ) А ) ) ) 0 1 2 3 4 5 6 7 8 найдет индексы: 0 1 3 4 7 8 начнем с постановки задачи, ваш код выводит 3 4. Но никак не 0 1 3 4 7 8 далее, на (((()A))((()))) вообще выведет кашу. полагаю правильный ответе - два индекса, 8 13 исходя из условия максимальной вложенности 3 4 будут проигнорированы Вот как только четко будут сформулированы условия, можно приступать к задаче, выбрав подходящий алгоритм без лишних ифов и тд Сообщение от Alone11 Но всегда ли можно обойтись регулярками скорее наоборот, всегда можно без них Добавлено через 5 минут Не по теме: Сообщение от o6938497 Почему бы через регулярки не распарсить? а сможете такую регулярку написать? :) а объяснить начинающему? 0

@КОП 1123 / 794 / 219 Регистрация: 15.08.2010 Сообщений: 2,185
	19.01.2019, 17:35	9
	Сообщение от Alone11 (((()A))((()))), все корректно отображается, но так как кол-во ( и ) не равное то первая скобка отмечается как корректная вы точно считали? Сообщение от Alone11 как делать корректный код сформулировать условие, которое себе не противоречит! Добавлено через 54 минуты Сообщение от Alone11 Lexeq, Написал более оптимизированный код это вы так подумали, на самом деле нет Сообщение от Alone11 как вариант норм. это вы так подумали, на самом деле мы еще не определились с задачей Сообщение от Alone11 Вы сказали что всегда можно без них, тоесть лучше if else вместо регулярок и в третий раз ваши домыслы. можно не значит лучше, но и не значит хуже. Сообщение от Alone11 мне рекомендовали выводиль любые if else в отдельные методы, верно ли это? нет. только когда это целесообразно. Основныепризнаки того что это ВОЗМОЖНО нужно сделать, это когда в ифе много строк кода и когда этот код переиспользуется 0

@Alone11 4 / 4 / 0 Регистрация: 14.10.2017 Сообщений: 85
	19.01.2019, 19:37 [ТС]	10
	Сообщение от КОП Сообщение от Alone11 (((()A))((()))), все корректно отображается, но так как кол-во ( и ) не равное то первая скобка отмечается как корректная вы точно считали? Блин, что то я не внимательный слишком. Когда себе код копировал, скопировал не до конца. Но в любом случае, теперь код работает та как надо. Добавлено через 7 минут Сообщение от КОП Сообщение от Alone11 как делать корректный код сформулировать условие, которое себе не противоречит! Не совсем понял в чем противоречие, ну тоесть я по не внимательности пропустил символ, может этим ввел в заблуждение. Но основной мотив именно в том как делать корректный код, как бы данный метод переписал тёртый мидл и выше, и главное почему. 0

@КОП 1123 / 794 / 219 Регистрация: 15.08.2010 Сообщений: 2,185
	19.01.2019, 20:25	11
	Сообщение от Alone11 Не совсем понял в чем противоречие сравним что выдает код с условием. Для удобства вместо индексов буду в исходной последовательности писать квадратные скобки. Сообщение от Alone11 Суть метода: найти индекс скобок внутри текста, в нутри которых нет ничего или только другие пустые скобки максимальной вложенности. Для `()(())` код выдаст все индексы. Согласно словесному условию, нужны скобки максимальной вложенности, то есть только эти `()[()]`, так как у них вложенность 2, а у других 1 - то есть просто пустые скобки. вторая часть `((a()))` ваш код выдаст обе крайние скобки `[[a()]]` однако `([a()])` содержит "а", а внешние скобки `[(a())]` содержат в себе другие непустые скобки 1

Опции темы