Реализация string.прототип.заменить


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

Спец

Spec

Код

static Func<string, RegExpParser.MatchState, string> MakeReplacer(string format)
{
    var tokens =
        new Regex(@"[^\$]*(\$[\$&`']|\$\d{1,2})")
        .Matches(format)
        .Cast<Match>()
        .Select(m => (Capture)m.Groups[1])
        .ToList();

    if (tokens.Count == 0)
    {
        return (value, state) => format;
    }

    var appendMethod = typeof(StringBuilder).GetMethod("Append", new[] { typeof(string) });
    var toStringMethod = typeof(StringBuilder).GetMethod("ToString", new Type[0]);
    var substringMethod = typeof(string).GetMethod("Substring", new[] { typeof(int) });
    var substringWithLengthMethod = typeof(string).GetMethod("Substring", new[] { typeof(int), typeof(int) });
    var zero = Expression.Constant(0);
    var one = Expression.Constant(1);
    var dollarSign = Expression.Constant("$");
    var valueVariable = Expression.Parameter(typeof(string), "value");
    var stateVariable = Expression.Parameter(typeof(RegExpParser.MatchState), "state");
    var sbVariable = Expression.Variable(typeof(StringBuilder), "sb");
    var capturesProp = Expression.Property(stateVariable, "captures");
    var capturesLengthProp = Expression.Property(capturesProp, "Length");
    var endIndexProp = Expression.Property(stateVariable, "endIndex");
    var inputProp = Expression.Property(stateVariable, "input");
    var appendDollarSign = Expression.Call(sbVariable, appendMethod, Expression.Constant("$"));
    var getMatchedSubstring = Expression.ArrayIndex(capturesProp, zero);
    var appendMatchedSubstring = Expression.Call(sbVariable, appendMethod, getMatchedSubstring);
    var getBeforeSubstring = Expression.Call(inputProp, substringWithLengthMethod, zero, Expression.Subtract(endIndexProp, one));
    var appendBeforeSubstring = Expression.Call(sbVariable, appendMethod, getBeforeSubstring);
    var getAfterSubstring = Expression.Call(inputProp, substringMethod, endIndexProp);
    var appendAfterSubstring = Expression.Call(sbVariable, appendMethod, getAfterSubstring);
    var e = (Expression)Expression.New(typeof(StringBuilder));
    var index = 0;

    foreach (var token in tokens)
    {
        if (token.Index > 0)
        {
            e = Expression.Call(e, appendMethod, Expression.Constant(format.Substring(index, token.Index - index)));                   
        }
        index = token.Index + token.Length;
        if (token.Value == "$$")
        {
            e = Expression.Call(e, appendMethod, dollarSign);
        }
        else if (token.Value == "$&")
        {
            e = Expression.Call(e, appendMethod, valueVariable);
        }
        else if (token.Value == "$`")
        {
            e = Expression.Call(e, appendMethod, getBeforeSubstring);
        }
        else if (token.Value == "$'")
        {
            e = Expression.Call(e, appendMethod, getAfterSubstring);
        }
        else
        {
            var n = token.Value.Substring(1);
            if (n.Length == 2 && n[0] == '0')
            {
                n = n.Substring(1);
            }
            var i = int.Parse(n) - 1;
            var nConst = Expression.Constant(i);
            var t = Expression.Constant(token.Value);
            if (i < 0)
            {
                e = Expression.Call(e, appendMethod, t);  
            }
            else
            {
                var cond =
                    Expression.Condition(
                        Expression.GreaterThan(
                            capturesLengthProp,
                            nConst
                        ),
                        Expression.ArrayIndex(capturesProp, nConst),
                        t                                
                    );
                e = Expression.Call(e, appendMethod, cond);  
            }
        }
    }

    e = Expression.Call(e, toStringMethod);
    var lambda = Expression.Lambda<Func<string, RegExpParser.MatchState, string>>(e, valueVariable, stateVariable);
    return lambda.Compile();
}

IDynamic Replace(IEnvironment environment, IArgs args)
{
    var o = environment.Context.ThisBinding;
    var so = o.ConvertToString();
    var s = so.BaseValue;
    var matches = new List<Tuple<int, int, Func<string, string>>>();
    var searchValueArg = args[0];
    var replaceValueArg = args[1];
    var replaceValueFunc = replaceValueArg as ICallable;
    var replaceValueString = replaceValueArg.ConvertToString();

    if (searchValueArg is NRegExp)
    {
        var regExpObj = (NRegExp)searchValueArg;
        var matcher = regExpObj.RegExpMatcher;

        Func<int, RegExpParser.MatchState, Func<string, string>> makeReplacer = null;

        if (replaceValueFunc != null)
        {
            makeReplacer =
                (startIndex, state) =>
                    _ =>
                    {
                        var nullArg = (IDynamic)environment.Null;
                        var cArgs = new List<IDynamic>();
                        foreach (var c in state.captures)
                        {
                            if (c == null)
                            {
                                cArgs.Add(environment.Null);
                                continue;
                            }
                            cArgs.Add(c == null ? nullArg : environment.CreateString(c));
                        }
                        cArgs.Add(environment.CreateNumber(startIndex));
                        cArgs.Add(so);
                        var result = 
                            replaceValueFunc.Call(
                                environment, 
                                environment.Undefined, 
                                environment.CreateArgs(cArgs)
                            );
                        return result.ConvertToString().BaseValue;
                    };
        }
        else
        {
            var replacer = MakeReplacer(replaceValueString.BaseValue);
            makeReplacer =
                (_, state) =>
                    v => replacer(v, state);
        }

        if (!regExpObj.Flags.Contains("g"))
        {
            var index = 0;
            do
            {
                var r = matcher(s, index);
                if (!r.success)
                {
                    index++;
                    continue;
                }
                matches.Add(
                    Tuple.Create(
                        index,
                        r.matchState.endIndex - index,
                        makeReplacer(index, r.matchState)
                    )
                );
                break;
            } while (index < s.Length);
        }
        else
        {
            var index = (int)regExpObj.Get("lastIndex").ConvertToNumber().BaseValue;
            do
            {
                var r = matcher(s, index);
                if (!r.success)
                {
                    index++;
                    continue;
                }
                matches.Add(
                    Tuple.Create(
                        index,
                        r.matchState.endIndex - index,
                        makeReplacer(index, r.matchState)                                
                    )
                );
                index = r.matchState.endIndex;
                regExpObj.Put("lastIndex", environment.CreateNumber(index), false);
            } while (index < s.Length);
        }
    }
    else
    {
        Func<string, string> replace = null;

        if (replaceValueFunc == null)
        {
            replace =  _ => replaceValueString.BaseValue;
        }
        else
        {
            replace = 
                v =>
                {
                    var arg = environment.CreateString(v);
                    var result =
                        replaceValueFunc.Call(
                            environment,
                            environment.Undefined,
                            environment.CreateArgs(new[] { arg })
                        );
                    return result.ConvertToString().BaseValue;
                };
        }

        var searchValue = args[0].ConvertToString().BaseValue;
        var index = 0;

        do
        {
            var resultIndex = s.IndexOf(searchValue, index);
            if (resultIndex < 0)
            {
                index++;
                continue;
            }
            matches.Add(
                Tuple.Create<int, int, Func<string, string>>(
                    resultIndex, 
                    searchValue.Length,
                    replace
                )
            );
            index = resultIndex + 1;
        } while (index < s.Length);
    }

    if (matches.Count == 0)
        return so;

    var sb = new StringBuilder();
    var i = 0;
    foreach (var match in matches)
    {
        if (match.Item1 > 0) 
            sb.Append(s.Substring(i, match.Item1 - i));
        sb.Append(match.Item3(s.Substring(match.Item1, match.Item2)));
        i = match.Item1 + match.Item2;
    }
    if (i < s.Length)
        sb.Append(s.Substring(i));
    return environment.CreateString(sb.ToString());                
}


716
5
c#
задан 7 апреля 2011 в 04:04 Источник Поделиться
Комментарии
1 ответ

Моей первой мыслью было “почему вы строите все эти выражения, только чтобы скомпилировать его и вернуть функции — почему бы просто не использовать лямбда-выражения, непосредственно”... но после раздумий кажется, что это не было бы яснее, поэтому я думаю, что с помощью выражений здесь на самом деле очень приятно :)

Вещи я замечаю, что может быть улучшено:

Вместо

var tokens =
new Regex(@"[^\$]*(\$[\$&`']|\$\d{1,2})")
.Matches(format)
.Cast<Match>()
.Select(m => (Capture)m.Groups[1])
.ToList();

Я думаю, вы могли бы написать

var items =
Regex.Matches(format, @"([^\$]*)(\$[\$&`']|\$\d{1,2})")
.Cast<Match>()
.Select(m => new { TextBefore = m.Groups[1].Value,
Token = m.Groups[2].Value });

Обратите внимание, что я изменил регулярное выражение при создании экземпляра объекта статического вызова метода; это мало что меняет, я просто думаю, что это более читабельным. Я также удалить вызов вызова метода toList() потому что вы, кажется, только выполняет итерации по этому, вы не манипулирует списке, так что нет никакой необходимости для этого дополнительный список экземпляров.

Теперь вам не нужно заботиться об индексе и можете избавиться от этой переменной:

foreach (var item in items)
{
if (item.TextBefore.Length > 0)
{
e = Expression.Call(e, appendMethod, Expression.Constant(item.TextBefore));
}

// ... etc.

Следующая вещь, которую я замечаю:

var n = token.Value.Substring(1);
if (n.Length == 2 && n[0] == '0')
{
n = n.Substring(1);
}
var i = int.Parse(n) - 1;

Вам не нужно, чтобы удалить ведущие нули. Он не собирается бросать инт.Разобрать выкл (это не означает восьмеричной, если это то, что вы подумали). Так можно просто писать:

var i = int.Parse(item.Token.Substring(1)) - 1;

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

Что касается второй способ...

var nullArg = (IDynamic)environment.Null;
var cArgs = new List<IDynamic>();
foreach (var c in state.captures)
{
if (c == null)
{
cArgs.Add(environment.Null);
continue;
}
cArgs.Add(c == null ? nullArg : environment.CreateString(c));
}

Это только мне кажется, или все это просто окольный способ сказать

var cArgs = state.captures
.Select(c => c == null ? (IDynamic)environment.Null
: environment.CreateString(c))
.ToList();

Слишком много локальных переменных, которые используются только один раз не всегда улучшения читабельности. Читатель не может сразу сказать, что переменная используется только один раз, поэтому это не очевидно, какие переменные они должны держать в голове во время чтения. Вот почему я подставлена nullArg. Если вы не согласны, то конечно вы можете изменить его обратно в локальную переменную.

В случае, если (!regExpObj.Флаги.Содержит("г")) заблокировать вас, кажется, есть немного дублирования кода. Мне кажется, что она легко может быть преобразована:

var globalFlag = regExpObj.Flags.Contains("g");
var index = globalFlag ? (int)regExpObj.Get("lastIndex").ConvertToNumber().BaseValue : 0;
do
{
var r = matcher(s, index);
if (!r.success)
{
index++;
continue;
}
matches.Add(
Tuple.Create(
index,
r.matchState.endIndex - index,
makeReplacer(index, r.matchState)
)
);

if (!globalFlag)
break;

index = r.matchState.endIndex;
regExpObj.Put("lastIndex", environment.CreateNumber(index), false);
}
while (index < s.Length);

Глядя на это снова, однако, я начинаю удивляться, почему вы не населяющие Играм список; вы не вернуть его или иным образом передает его в любом месте. Похоже, что вы на самом деле нужно только итоговую строку, а не список игр, верно? Если это так, то лично я бы, наверное, просто создать строку и не заморачиваться со списком матчей. Это также позволяет избавиться от проблемы я вижу в использовании кортежей: имена место № 1, место № 2, Item3 не очень читаемый.

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

var resultIndex = s.IndexOf(searchValue, index);
if (resultIndex < 0)
{
index++;
continue;
}

Вы ищете одну и ту же строку для той же подстроки много раз, даже когда вы знаете, что это не существует. Если resultIndex < 0 вы можете остановить:

var resultIndex = s.IndexOf(searchValue, index);
if (resultIndex < 0)
break;

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

Я считаю, что это серьезная ошибка:

index = resultIndex + 1;

Я думаю, что вы хотите этим сказать

index = resultIndex + searchValue.Length;

Представьте, клиент хочет заменить все экземпляры АА с х и входная строка содержит аааа. Если я читаю код в конце правильно (код, который использует Элемент1/место № 2/Item3 много), он будет бросать, потому что ты бы передавая отрицательное значение в подстроке.

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

2
ответ дан 8 апреля 2011 в 01:04 Источник Поделиться