Тесты строки в C # - Рефакторинг для скорости / ремонтопригодности

Question

Я в свое время возился с небольшими функциями, пытаясь найти способы их рефакторинга (недавно я читал книгу Мартина Фаулера Рефакторинг: улучшение дизайна существующего кода ). Я обнаружил следующую функцию MakeNiceString() при обновлении другой части кодовой базы рядом с ней, и это выглядело как хороший кандидат, с которым можно связываться. На самом деле, нет никакой реальной причины заменить его, но он достаточно мал и делает что-то маленькое, так что за ним легко следовать, и все же получить «хороший» опыт.

private static string MakeNiceString(string str)
        {
            char[] ca = str.ToCharArray();
            string result = null;
            int i = 0;
            result += System.Convert.ToString(ca[0]);
            for (i = 1; i <= ca.Length - 1; i++)
            {
                if (!(char.IsLower(ca[i])))
                {
                    result += " ";
                }
                result += System.Convert.ToString(ca[i]);
            }
            return result;
        }


static string SplitCamelCase(string str)
    {
        string[] temp = Regex.Split(str, @"(?<!^)(?=[A-Z])");
        string result = String.Join(" ", temp);
        return result;
    }

Первая функция MakeNiceString() - это функция, которую я нашел в коде, который я обновлял на работе. Цель этой функции - перевести ThisIsAString в This String . Он используется в полдюжине мест в коде и довольно незначителен для всей схемы вещей.

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

Ну, вот результаты:

С 10 итерациями:

MakeNiceString took 2649 ticks
SplitCamelCase took 2502 ticks

Тем не менее, он сильно меняется в течение долгого пути:

С 10 000 итераций:

MakeNiceString took 121625 ticks
SplitCamelCase took 443001 ticks

---

Рефакторинг MakeNiceString()

Процесс рефакторинга MakeNiceString() начался с простого удаления происходящих преобразований. Это дало следующие результаты:

MakeNiceString took 124716 ticks
ImprovedMakeNiceString took 118486

Вот код после Refactor # 1:

private static string ImprovedMakeNiceString(string str)
        { //Removed Convert.ToString()
            char[] ca = str.ToCharArray();
            string result = null;
            int i = 0;
            result += ca[0];
            for (i = 1; i <= ca.Length - 1; i++)
            {
                if (!(char.IsLower(ca[i])))
                {
                    result += " ";
                }
                result += ca[i];
            }
            return result;
        }

Refactor # 2 - Use StringBuilder

Моей второй задачей было использовать StringBuilder вместо String. Поскольку String является неизменным, ненужные копии создавались по всему циклу. Ниже приводится эталон для использования, а также код:

static string RefactoredMakeNiceString(string str)
        {
            char[] ca = str.ToCharArray();
            StringBuilder sb = new StringBuilder((str.Length * 5 / 4));
            int i = 0;
            sb.Append(ca[0]);
            for (i = 1; i <= ca.Length - 1; i++)
            {
                if (!(char.IsLower(ca[i])))
                {
                    sb.Append(" ");
                }
                sb.Append(ca[i]);
            }
            return sb.ToString();
        }

В результате получается следующий эталонный тест:

MakeNiceString Took:           124497 Ticks   //Original
SplitCamelCase Took:           464459 Ticks   //Regex
ImprovedMakeNiceString Took:   117369 Ticks   //Remove Conversion
RefactoredMakeNiceString Took:  38542 Ticks   //Using StringBuilder

Изменение цикла for на цикл foreach привело к следующему результату теста:

static string RefactoredForEachMakeNiceString(string str)
        {
            char[] ca = str.ToCharArray();
            StringBuilder sb1 = new StringBuilder((str.Length * 5 / 4));
            sb1.Append(ca[0]);
            foreach (char c in ca)
            {
                if (!(char.IsLower(c)))
                {
                    sb1.Append(" ");
                }
                sb1.Append(c);
            }
            return sb1.ToString();
        }

RefactoredForEachMakeNiceString    Took:  45163 Ticks

Как видите, с точки зрения обслуживания цикл foreach будет самым простым в обслуживании и будет иметь «самый чистый» вид. Это немного медленнее, чем цикл for, но бесконечно легче следовать.

Альтернативный Рефактор: Использовать Скомпилировано Regex

Я передвинул Regex вправо перед началом цикла, в надежде, что, поскольку он компилируется только один раз, он будет выполняться быстрее. Что я обнаружил (и я уверен, что у меня где-то есть ошибка), так это то, что это не происходит так, как должно:

static void runTest5()
        {
            Regex rg = new Regex(@"(?<!^)(?=[A-Z])", RegexOptions.Compiled);
            for (int i = 0; i < 10000; i++)
            {
                CompiledRegex(rg, myString);
            }
        }
 static string CompiledRegex(Regex regex, string str)
    {
        string result = null;
        Regex rg1 = regex;
        string[] temp = rg1.Split(str);
        result = String.Join(" ", temp);
        return result;
    }

Итоговые результаты теста:

MakeNiceString Took                   139363 Ticks
SplitCamelCase Took                   489174 Ticks
ImprovedMakeNiceString Took           115478 Ticks
RefactoredMakeNiceString Took          38819 Ticks
RefactoredForEachMakeNiceString Took   44700 Ticks
CompiledRegex Took                    227021 Ticks

Или, если вы предпочитаете миллисекунды:

MakeNiceString Took                  38 ms
SplitCamelCase Took                 123 ms
ImprovedMakeNiceString Took          33 ms
RefactoredMakeNiceString Took        11 ms
RefactoredForEachMakeNiceString Took 12 ms
CompiledRegex Took                   63 ms

Таким образом, процентное увеличение составляет:

MakeNiceString                   38 ms   Baseline
SplitCamelCase                  123 ms   223% slower
ImprovedMakeNiceString           33 ms   13.15% faster
RefactoredMakeNiceString         11 ms   71.05% faster
RefactoredForEachMakeNiceString  12 ms   68.42% faster
CompiledRegex                    63 ms   65.79% slower

(пожалуйста, проверьте мою математику)

В конце я собираюсь заменить то, что там, на RefactoredForEachMakeNiceString(), и пока я в нем, я собираюсь переименовать его во что-нибудь полезное, например SplitStringOnUpperCase.

Контрольный тест:

Для сравнения я просто вызываю новый Stopwatch для каждого вызова метода:

       string myString = "ThisIsAUpperCaseString";
       Stopwatch sw = new Stopwatch();
       sw.Start();
       runTest();
       sw.Stop();

     static void runTest()
        {

            for (int i = 0; i < 10000; i++)
            {
                MakeNiceString(myString);
            }


        }

---

Вопросы

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

---

Спасибо за ваши ответы до сих пор. Я вставил все предложения, сделанные @Jon Skeet, и хотел бы получить отзыв об обновленных вопросах, которые я задал в результате.

NB : Этот вопрос предназначен для изучения способов рефакторинга функций обработки строк в C #. Я скопировал / вставил первый код as is. Я хорошо знаю, что вы можете удалить System.Convert.ToString() в первом методе, и я сделал именно это. Если кто-либо знает о каких-либо последствиях удаления System.Convert.ToString(), это также будет полезно знать.

Answer 1

1) Используйте StringBuilder, предпочтительно с разумной начальной емкостью (например, длина строки * 5/4, чтобы разрешить один дополнительный пробел на четыре символа).

2) Попробуйте использовать цикл foreach вместо цикла for - это может быть проще

3) Вам не нужно сначала преобразовывать строку в массив символов - foreach уже будет работать над строкой или использовать индексатор.

4) Не делайте дополнительных преобразований строк везде - вызывайте Convert.ToString (char) и затем добавляйте эту строку бессмысленно; нет необходимости в одной строке символов

5) Для второго варианта просто создайте регулярное выражение один раз, вне метода. Попробуйте также с RegexOptions.Compiled.

РЕДАКТИРОВАТЬ: Хорошо, полные результаты тестов. Я перепробовал еще несколько вещей, а также выполнил код с большим количеством итераций, чтобы получить более точный результат. Это работает только на Eee PC, поэтому, без сомнения, он будет работать быстрее на «настоящих» ПК, но я подозреваю, что широкие результаты уместны. Сначала код:

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Text;
using System.Text.RegularExpressions;

class Benchmark
{
    const string TestData = "ThisIsAUpperCaseString";
    const string ValidResult = "This Is A Upper Case String";
    const int Iterations = 1000000;

    static void Main(string[] args)
    {
        Test(BenchmarkOverhead);
        Test(MakeNiceString);
        Test(ImprovedMakeNiceString);
        Test(RefactoredMakeNiceString);
        Test(MakeNiceStringWithStringIndexer);
        Test(MakeNiceStringWithForeach);
        Test(MakeNiceStringWithForeachAndLinqSkip);
        Test(MakeNiceStringWithForeachAndCustomSkip);
        Test(SplitCamelCase);
        Test(SplitCamelCaseCachedRegex);
        Test(SplitCamelCaseCompiledRegex);        
    }

    static void Test(Func<string,string> function)
    {
        Console.Write("{0}... ", function.Method.Name);
        Stopwatch sw = Stopwatch.StartNew();
        for (int i=0; i < Iterations; i++)
        {
            string result = function(TestData);
            if (result.Length != ValidResult.Length)
            {
                throw new Exception("Bad result: " + result);
            }
        }
        sw.Stop();
        Console.WriteLine(" {0}ms", sw.ElapsedMilliseconds);
        GC.Collect();
    }

    private static string BenchmarkOverhead(string str)
    {
        return ValidResult;
    }

    private static string MakeNiceString(string str)
    {
        char[] ca = str.ToCharArray();
        string result = null;
        int i = 0;
        result += System.Convert.ToString(ca[0]);
        for (i = 1; i <= ca.Length - 1; i++)
        {
            if (!(char.IsLower(ca[i])))
            {
                result += " ";
            }
            result += System.Convert.ToString(ca[i]);
        }
        return result;
    }

    private static string ImprovedMakeNiceString(string str)
    { //Removed Convert.ToString()
        char[] ca = str.ToCharArray();
        string result = null;
        int i = 0;
        result += ca[0];
        for (i = 1; i <= ca.Length - 1; i++)
        {
            if (!(char.IsLower(ca[i])))
            {
                result += " ";
            }
            result += ca[i];
        }
        return result;
    }

    private static string RefactoredMakeNiceString(string str)
    {
        char[] ca = str.ToCharArray();
        StringBuilder sb = new StringBuilder((str.Length * 5 / 4));
        int i = 0;
        sb.Append(ca[0]);
        for (i = 1; i <= ca.Length - 1; i++)
        {
            if (!(char.IsLower(ca[i])))
            {
                sb.Append(" ");
            }
            sb.Append(ca[i]);
        }
        return sb.ToString();
    }

    private static string MakeNiceStringWithStringIndexer(string str)
    {
        StringBuilder sb = new StringBuilder((str.Length * 5 / 4));
        sb.Append(str[0]);
        for (int i = 1; i < str.Length; i++)
        {
            char c = str[i];
            if (!(char.IsLower(c)))
            {
                sb.Append(" ");
            }
            sb.Append(c);
        }
        return sb.ToString();
    }

    private static string MakeNiceStringWithForeach(string str)
    {
        StringBuilder sb = new StringBuilder(str.Length * 5 / 4);
        bool first = true;      
        foreach (char c in str)
        {
            if (!first && char.IsUpper(c))
            {
                sb.Append(" ");
            }
            sb.Append(c);
            first = false;
        }
        return sb.ToString();
    }

    private static string MakeNiceStringWithForeachAndLinqSkip(string str)
    {
        StringBuilder sb = new StringBuilder(str.Length * 5 / 4);
        sb.Append(str[0]);
        foreach (char c in str.Skip(1))
        {
            if (char.IsUpper(c))
            {
                sb.Append(" ");
            }
            sb.Append(c);
        }
        return sb.ToString();
    }

    private static string MakeNiceStringWithForeachAndCustomSkip(string str)
    {
        StringBuilder sb = new StringBuilder(str.Length * 5 / 4);
        sb.Append(str[0]);
        foreach (char c in new SkipEnumerable<char>(str, 1))
        {
            if (char.IsUpper(c))
            {
                sb.Append(" ");
            }
            sb.Append(c);
        }
        return sb.ToString();
    }

    private static string SplitCamelCase(string str)
    {
        string[] temp = Regex.Split(str, @"(?<!^)(?=[A-Z])");
        string result = String.Join(" ", temp);
        return result;
    }

    private static readonly Regex CachedRegex = new Regex("(?<!^)(?=[A-Z])");    
    private static string SplitCamelCaseCachedRegex(string str)
    {
        string[] temp = CachedRegex.Split(str);
        string result = String.Join(" ", temp);
        return result;
    }

    private static readonly Regex CompiledRegex =
        new Regex("(?<!^)(?=[A-Z])", RegexOptions.Compiled);    
    private static string SplitCamelCaseCompiledRegex(string str)
    {
        string[] temp = CompiledRegex.Split(str);
        string result = String.Join(" ", temp);
        return result;
    }

    private class SkipEnumerable<T> : IEnumerable<T>
    {
        private readonly IEnumerable<T> original;
        private readonly int skip;

        public SkipEnumerable(IEnumerable<T> original, int skip)
        {
            this.original = original;
            this.skip = skip;
        }

        public IEnumerator<T> GetEnumerator()
        {
            IEnumerator<T> ret = original.GetEnumerator();
            for (int i=0; i < skip; i++)
            {
                ret.MoveNext();
            }
            return ret;
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }
    }
}

Теперь результаты:

BenchmarkOverhead...  22ms
MakeNiceString...  10062ms
ImprovedMakeNiceString...  12367ms
RefactoredMakeNiceString...  3489ms
MakeNiceStringWithStringIndexer...  3115ms
MakeNiceStringWithForeach...  3292ms
MakeNiceStringWithForeachAndLinqSkip...  5702ms
MakeNiceStringWithForeachAndCustomSkip...  4490ms
SplitCamelCase...  68267ms
SplitCamelCaseCachedRegex...  52529ms
SplitCamelCaseCompiledRegex...  26806ms

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

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

Answer 2

Возможно, вы захотите создать экземпляр объекта Regex в качестве члена класса и использовать опцию RegexOptions.Compiled при его создании.

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

Answer 3

Моим первым рефакторингом будет изменение имени метода на более описательное. MakeNiceString imo - это не имя, которое бы указывало на то, что делает этот метод.

Как насчет PascalCaseToSentence? Не люблю это имя, но оно лучше, чем MakeNiceString.

Answer 4

Я знаю, что они говорят о RegEx, использую его для решения проблемы, и теперь у вас есть две проблемы, но я остаюсь фанатом, Просто для ухмылок, вот версия RegEx. RegEx с небольшим количеством инициации легко читается, меньше кода и позволяет легко добавлять дополнительные разделители (как я делал с запятой).

   s1 = MakeNiceString( "LookOut,Momma,There'sAWhiteBoatComingUpTheRiver" ) );

   private string MakeNiceString( string input )
   {
       StringBuilder sb = new StringBuilder( input );
       int Incrementer = 0;
       MatchCollection mc;
       const string SPACE = " ";

       mc = Regex.Matches( input, "[A-Z|,]" );

       foreach ( Match m in mc )
       {
           if ( m.Index > 0 )
           {
               sb.Insert( m.Index + Incrementer, SPACE );
               Incrementer++;
           }
       }

       return sb.ToString().TrimEnd();
   }

Answer 5

Это в ответ на комментарий ctacke к ответу Джона Скита (это не долго для комментария)

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

На самом деле нет, в этом случае foreach будет быстрее. Доступ к индексу проверяется границами (т. Е. Я проверяю, чтобы быть в диапазоне три раза в цикле: один раз в for () и один раз для двух ca [i] s), что делает цикл for медленнее, чем foreach.

Если компилятор C # обнаруживает определенный синтаксис:

 for(i = 0; i < ca.Length; i++)

тогда он выполнит специальную оптимизацию, удалив внутренние проверки границ, сделав цикл for () быстрее. Однако, поскольку здесь мы должны рассматривать ca [0] как особый случай (чтобы предотвратить вывод пробела на выходе), мы не можем запустить эту оптимизацию.

Answer 6

Попробуйте рефакторинг, чтобы регулярное выражение, которое вы используете для разделения строки во втором методе, было сохранено в статическом методе и было построено с использованием параметра RegexOptions.Compiled. Подробнее об этом здесь: http://msdn.microsoft.com/en-us/library/8zbs0h2f.aspx.

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

Answer 7

Используйте StringBuilder вместо конкатенации. Каждая конкатенация создает новый экземпляр строки и выбрасывает старый.

Answer 8

Вот немного более оптимальный вариант. Я принимал предложения от предыдущих постеров, но также добавлял к строителю строк блочным способом. Это может позволить сборщику строк копировать 4 байта за раз, в зависимости от размера слова. Я также удалил распределение строк и просто заменил его на str.length.

    static string RefactoredMakeNiceString2(string str)
    {
        char[] ca = str.ToCharArray();
        StringBuilder sb = new StringBuilder(str.Length);
        int start = 0;
        for (int i = 0; i < ca.Length; i++)
        {
            if (char.IsUpper(ca[i]) && i != 0)
            {
                sb.Append(ca, start, i - start);
                sb.Append(' ');
                start = i;
            }
        }
        sb.Append(ca, start, ca.Length - start);
        return sb.ToString();
    }

Answer 9

Regex-версии вашего решения не эквивалентны в результатах исходному коду. Возможно, более широкий контекст кода избегает областей, где они различаются. Исходный код добавит пробел для всего, что не является символом нижнего регистра. Например, "This\tIsATest" станет "This \t Is A Test" в оригинале, но "This\t Is A Test" с версиями Regex.

(?<!^)(?=[^a-z])

Это шаблон, который вы хотите для более близкого соответствия, но даже тогда он по-прежнему игнорирует проблемы i18n. Об этом должен позаботиться следующий шаблон:

(?<!^)(?=\P{Ll})

Answer 10

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

</p> <p>string A = B + C;</p> <p>A += D; A += E;</p> <p>// ... rinse-repeat for 10,000 iterations</p> <p>

Для каждой строки выше .NET будет: 1) Выделите немного новой памяти для А. 2) Скопируйте строку B в новую память. 3) Расширить память, чтобы держать C. 4) Добавьте строку C к A.

Чем длиннее строка A, тем больше времени это занимает. Добавьте к этому, чем больше раз вы делаете это, тем дольше становится A, экспоненциально дольше это занимает.

Однако в StringBuilder вы не выделяете новую память, поэтому вы пропускаете эту проблему.

Если вы скажете:

</p> <pre><code>StringBuilder A = new StringBuilder(); A.Append(B); A.Append(C); // .. rinse/repeat for 10,000 times... string sA = A.ToString();

StringBuilder (Edit: fixed description) содержит строку в памяти. Не нужно перераспределять всю строку для каждой добавленной подстроки. Когда вы запускаете ToString (), строка уже добавляется в правильном формате.

Один выстрел вместо цикла, который занимает все более длительный период.

Надеюсь, это поможет ответить на вопрос, ПОЧЕМУ это заняло гораздо меньше времени.