Каковы некоторые индикаторы, которые я слишком задумывался над моим решением этой проблемы?

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

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

В то время как некоторая обратная связь противоречит другим отзывам, мне не раз говорили о том, что я переусердствую и что мои решения слишком сложны (не говоря о сложности, как в Big O). Я пытаюсь понять, что это значит, поэтому я знаю, как и что менять.

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

Вопросы, которые у меня есть (это, где мне нужна ваша помощь): Что подразумевается под избыточным или чрезмерно сложным? Любые примеры? Что было бы хорошим подходом, чтобы избежать этого?

Я приведу вам пример, и имейте в виду, что код был написан под давлением, и я был нервным крушением. Одна из вещей, которую я должен был реализовать, заключалась в замене кода страны на телефонный номер на 0 и удалении места и не didgit (это было из примерно 10 рассказов пользователей). Я сделал это в стиле TDD, и вот тест:

        [TestMethod]
    public void StripNumber_WhenGivenNumber_ReturnsNumberWithoutSpaceAndNonNumeric()
    {
        var number = "046 55.5";
        var expectedString = "046555";

        var numberManager = new PhoneNumberManager(number);
        var cleanedString = numberManager.StripWhiteSpaceAndNonDigit().Result();

        Assert.AreEqual(expectedString, cleanedString);
    }

И код после быстрого рефакторинга.

    internal class PhoneNumberManager
{
    private string _nonNumeric = "[^0-9.]";
    private string _result;

    public PhoneNumberManager(string number)
    {
        _result = number;
    }

    public PhoneNumberManager StripWhiteSpaceAndNonDigit()
    {
        _result = Regex.Replace(_result, _nonNumeric, "").Replace(".", "");
        return this;
    }
    //bunch of other methods, and a Result() method
}

Что /как я сделал:

  • Начинается тестирование
  • Написал достаточно кода, чтобы он прошел - краткий тест между ними, сделал некоторые отладки
  • Рефакторинг для класса
  • Проверенный тест по-прежнему работает зеленый
  • Быстрый мелкий рефакторинг
  • Перешел к следующей истории

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

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

EDIT: поскольку некоторые, похоже, пропустили часть нескольких рассказов пользователей: их было 10, и все они имеют дело с номером телефона.

ИЗМЕНИТЬ RE. TDD: интервьюер много говорил о TDD перед интервью (и как это было важно для них), и до того, как я начал кодирование, я спросил, следует ли мне делать TDD, так как они сказали, что меня будут оценивать по коду и тестированию. Ответ был: «делайте то, что вы обычно делаете», поэтому я сделал TDD.

Поскольку я был кодирующим, я сказал такие вещи, как:

  • Я проверил количество данных, с которыми мы будем работать, чтобы оценить, насколько я должен беспокоиться о сложности.
  • Именование не ужасно хорошо здесь, но я оставлю его на этом пока, так как фокус - это код (я просто забыл о наименовании)
  • Мне не нравится комманда Regex и Replace, я должен использовать только Regex
  • Другой разработчик (интервью) предложил матч с регулярным выражением, но я так и не сказалэто означало, что мы получим раскол в каждой незначительной цифре, получив коллекцию, которая, как я думал, добавит дополнительные служебные данные.
  • Мне не понравился одинокий â € œ0â € concat
  • Я сожалел, что вместо этого не использовал построитель строк, но количество данных не было бы большим (первое, что я спросил)

В конце сеанса кодирования я реорганизовал класс на свободный подход с привязкой к методу, объяснив, что я считал это конвейерным подходом, где мы могли бы использовать разные каналы и все еще иметь одно определенное место для выход.

Где я ошибся? Каковы признаки чрезмерного мышления в коде? - а также всегда рады услышать, что испорчено в коде или может быть улучшено :)

29 голосов | спросил Iris Classon 5 J000000Saturday14 2014, 00:10:36

10 ответов


13

Единственным признаком чрезмерного мышления было ваше использование бизнес-примитива (PhoneNumberManager).

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

Похоже, это также заставило вас пропустить очевидный подход к расширению строки.

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

, поэтому код мудрый:

public static class PhoneNumberExtensions
{
    private static readonly Regex NonDigit = new Regex(@"\D", RegexOptions.Compiled);

    public static string StripWhiteSpaceAndNonDigit(this string phoneNumber)
    {
        return NonDigit.Replace(phoneNumber, string.Empty);
    }

    public static string RemoveDialingCode(this string phoneNumber)
    {
    .....
    }
}
ответил Graeme Bradbury 5 J000000Saturday14 2014, 00:59:40
20

выражение Regex [^0-9.] соответствует любому, кроме 0, 9 и . , Если вы выберете . из выражения, оно должно соответствовать только вам. Но это также может быть упрощено до \D, который соответствует чему угодно, кроме цифр.

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

Regex.Replace(value, @"\D", "")

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

internal class PhoneNumberHelper
{
    public static string StripNonDigit(string number)
    {
        const string NonDigit = @"\D";
        return Regex.Replace(number, NonDigit, string.Empty);
    }

    // and your bunch of other methods below
}

  

Предварительная оптимизация - это корень всех злых

Хотя ваш код может быть не идеальным или оптимизированным, но он работает, и это ваш основной фокус /цель. Оптимизация и рефакторинг происходят только после того, как во время работы над кодом ваше мнение сужается по этой проблеме. И, таким образом, не удалось просмотреть общую картину.

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

ответил Xiaoy312 5 J000000Saturday14 2014, 00:34:33
13

Когда вы в интервью, несколько интервьюеров ожидают, что вы придумаете идеальное решение. В основном, они ищут представление о том, как вы думаете, и как вы приближаетесь к проблеме. Особенно, как младший программист.

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

Мне нравится «когда у вас молот, все выглядит как гвоздь». У вас есть целый набор инструментов, который вы создали за относительно короткий промежуток времени, и вам естественно придётся применить некоторые из этих материалов.

Что касается индикаторов, вы переусердствуете,

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

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

ответил Hammerstein 5 J000000Saturday14 2014, 00:30:18
8

Это было бы легче читать:

    public static string GetFormattedPhoneNumber(string input)
    {
        StringBuilder result = new StringBuilder();

        foreach (char character in input)
        {
            if (Char.IsDigit(character))
            {
                result.Append(character);
            }
        }

        return result.ToString();
    }
ответил ArchieCoder 5 J000000Saturday14 2014, 00:27:04
7

Ваше регулярное выражение удаляет все, что не является 0-9 или «.», а затем вы заменяете «.». Вы могли бы удалить '.' из регулярного выражения, чтобы сделать все за один снимок:

"[^ 0-9]" вместо "[^ 0-9.]"

Не забудьте ответить на заданный вопрос. Вы упростили число, но необходимо получить код страны.

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

Я бы не принял подход TDD. У меня была бы одна функция, которая может быть вызвана для вывода результата (возможно, с поддерживающими функциями или классами).

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

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

ответил Will 5 J000000Saturday14 2014, 00:25:56
6

Один из основных «чрезмерных значений» для меня - это return this;

Я лично использую это в основном для шаблона Builder. Поскольку ваш класс называется PhoneNumberManager, он не похож на PhoneNumberBuilder (возможно, переименовать?). Вы упомянули, что у вас есть и другие методы, поэтому главный вопрос: имеет ли смысл связывать их? Или вам нужно только позвонить одному или двум из них, а затем получить результат?

Если нет смысла их связывать, чего не видно по виду вашего кода в вашем тесте:

    var numberManager = new PhoneNumberManager(number);
    var cleanedString = numberManager.StripWhiteSpaceAndNonDigit().Result();

Затем var cleanedString = PhoneNumber.StripWhiteSpaceAndNonDigit(number); будет намного лучше, читабельно и менее сложно.


Другой подход, предполагающий, что все методы возвращают string и требует string, может использовать делегаты Func<string, string>. Тогда я считаю, что у вас может быть целая куча статических методов в вашем классе PhoneNumber и использовать один служебный метод:

public static string Process(IEnumerable<Func<string, string>> functions, string str) {
     string value = str;
     foreach (Func<string, string> func in functions) {
          value = func.apply(value);
     }
     return value;
}

И затем назовите его PhoneNumber.Process(PhoneNumber::StripWhiteSpaceAndNonDigits, PhoneNumber::StripStuff)

Возможно, у меня смешанный C # с синтаксисом Java. ( Старый Хорошие привычки умирают тяжело)


Я должен сказать, что вы, кажется, знаете свой C #, хотя, вы придерживаетесь всех конвенций C #, о которых я знаю, кроме, конечно, вашей частной строки private string _nonNumeric = "[^0-9.]"; должен быть readonly или const.

ответил Simon Forsberg 5 J000000Saturday14 2014, 01:32:10
4

Не уверен, что это проблема, потому что меня там не было (и пропустил все сообщение), но:

Если это требование:

  

Одна из вещей, которую я должен был реализовать, заключалась в замене кода страны на   номер телефона с 0 и удалить пробел и не цифру (это был один   из ок. 10 рассказов пользователей).

В тестовом примере вы должны объяснить это требование:

[TestMethod]
public void StripNumber_WhenGivenNumber_ReturnsNumberWithoutSpaceAndNonNumeric()
{
    var number = "046 55.5";
    var expectedString = "046555";

    var numberManager = new PhoneNumberManager(number);
    var cleanedString = numberManager.StripWhiteSpaceAndNonDigit().Result();

    Assert.AreEqual(expectedString, cleanedString);
}

(Я предполагаю, что у вас есть другой тестовый пример, объясняющий замену кода страны на 0 ?, который выполняется другим способом?)

Если я прочитаю это требование и напишу его в качестве одного теста, я получаю:

public class PhoneNumberTests
{
    public class Parse
    {
        [Fact]
        public void Must_parse_phone_number_to_a_correct_local_number()
        {
            // Arrange
            var phoneNumber = "+31 20.1234.567";

            // Act
            var result = PhoneNumber.Parse(phoneNumber);

            // Assert
            Assert.Equal("0201234567", result.LocalNumber);
        }
    }
}

Способ написания тестового кода не имеет значения, но посмотрите на разницу в разговоре с объектом.

(Мы можем спорить весь день, если статические методы хороши или плохи, но это не то, что я хочу указать.)

У вас нет объекта для описания номера телефона, но какой-то класс швейцарских армейских ножей, который может делать все, что вы просите его, на строку /номер телефона (я не знаю, было ли это требование).

Вы не можете задать вопрос: можете ли вы указать мне местный номер для этой строки?

Вам нужно спросить: не можете ли вы заменить пробелы для меня и удалить несимвольные символы, а также заменить код области на 0 для этой строки?

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

ответил Erik Lieben 5 J000000Saturday14 2014, 10:03:44
1

Вот несколько моментов, которые мне не нравятся в качестве интервьюера:

  • Имя тестового метода чрезвычайно абстрактно, просто назовите его Test
  • Имя PhoneNumberManager сбивает с толку для этой цели, PhoneNumberHelper будет более удобным
  • Вы инициализируете класс для выполнения простой задачи, а все, что вам нужно, - это один статический метод.
  • Вы должны немедленно вернуть переформатированный номер телефона.
  • Нет причин возвращать this, потому что, очевидно, он уже известен вызывающему
  • Вы используете регулярное выражение для изменения данных, в то время как вы даже не уверены, действительно ли данные действительны в первую очередь, но я просто предполагаю, что интервьюер не просил об этом явно.
  • Вы выполняете дополнительную операцию String.Replace, вы могли бы сделать это немедленно с помощью Regex.Replace
  • _nonNumeric должен был быть буквальным
ответил Leopold Asperger 6 J000000Sunday14 2014, 00:08:58
1

У вас уже есть много хороших ответов о дизайне вашего класса. (Это должен быть статический метод, принимающий строку в качестве аргумента и возвращающую строку, ничего другого, но вам уже сказали, что).

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

Здесь все, что вам нужно, для фильтрации строки. Так как строки неизменны в C # (что в данном случае является грустным, поскольку в противном случае вы могли бы изменить строку на месте, так как результирующая строка всегда будет одинаковой длины или короче), вам нужно создать новую строку, содержащую только некоторые символы в старый.

static string removeNonValid(string nbr)
{   
    StringBuilder sb = new StringBuilder();
    for (int i = 0; i < nbr.Length; i++)
    {   
        if (Char.IsDigit(nbr[i]))
        {   
            sb.Append(nbr[i]);
        }   
    }   
    return sb.ToString();
}   

Да, regexp будет работать здесь, так как regexp может все это сделать, но это переполнение (и намного медленнее).

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

ответил iveqy 9 PM00000040000001831 2014, 16:00:18
0
string Digits = (new PhoneNumberManagerFactory() as IPhoneNumberManagerFactory).Create<PhoneNumberManager>().StripNonDigits("123.456");

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

ответил m0rp 8 J000000Tuesday14 2014, 16:18:56

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132