Как оптимизировать код?

Question

[Макс Максимов]

Здравствуйте.

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

if (!string.IsNullOrEmpty(searchByName))
            {
                string[] names = searchByName.Split(' ');

                if (names.Count() > 1)
                {
                    var query = (
                        from i in MongoDBInstance.GetMongoDatabase.GetCollection<User>("UserInfo").AsQueryable<User>()
                        where
                            i.IsActivated == false && ((
                                i.Name.ToUpper().Contains(names[0].ToUpper()) &&
                                i.LastName.ToUpper().Contains(names[1].ToUpper())) ||
                            (i.Name.ToUpper().Contains(names[1].ToUpper()) &&
                             i.LastName.ToUpper().Contains(names[0].ToUpper())))
                        orderby i.ID
                        select i);
                    if (query.Count() > 0)
                    {
                        ViewData["uncheckedProfiles"] = query.ToPagedList(pageNumber, pageSize);
                    }
                    else
                    {
                        ViewData["uncheckedProfilesSearch"] = "NoResult";
                    }
                }
                else
                {
                    var query = (
                        from i in MongoDBInstance.GetMongoDatabase.GetCollection<User>("UserInfo").AsQueryable<User>()
                        where
                            i.IsActivated == false && (
                                i.Name.ToUpper().Contains(searchByName.ToUpper()) ||
                                i.LastName.ToUpper().Contains(searchByName.ToUpper()))
                        orderby i.ID
                        select i);

                    if (query.Count() > 0)
                    {
                        ViewData["uncheckedProfiles"] = query.ToPagedList(pageNumber, pageSize);
                    }
                    else
                    {
                        ViewData["uncheckedProfilesSearch"] = "NoResult";
                    }
                }
            }
            else
            {
                var query = (
                    from i in MongoDBInstance.GetMongoDatabase.GetCollection<User>("UserInfo").AsQueryable<User>()
                    where i.IsActivated == false
                    orderby i.ID
                    select i);

                if (query.Count() > 0)
                {
                    ViewData["uncheckedProfiles"] = query.ToPagedList(pageNumber, pageSize);
                }
                else
                {
                    ViewData["uncheckedProfiles"] = null;
                }
            }

Answer 1

[Алексей Кулаков]

Добавлю свою ложку дегтя плюсом к предыдущим ответам

1) query.Count() > 0 если это метод из Linq а не определенный в классе то лучше так не делать. Он переберет весь IEnumerable, а оно вам надо? Если нужно проверить есть ли в последовательности элементы используйте query.Any()

2)в if-ах можно определить только запрос. а далее просто

if (query.Any())
{
   ViewData["uncheckedProfiles"] = query.ToPagedList(pageNumber, pageSize);
}
else
{
   ViewData["uncheckedProfilesSearch"] = "NoResult";
}

или

3) думаю можно что-то сделать с вашими ToUpper(). например почему бы сразу не привести все что нужно (если оно нужно) к верхнему регистру и не делать лишних операций. потому что судя по описанию на MSDN ToUpper() выдает копию строки, то есть это как минимум каждый раз инициализация переменной и копирование из одной переменной в другую. А вообще почитайте про работу со строками с точки зрения внутреннего устройства, например рихтера "CLR via C#"

4) вообще судя по вашему запросу у вас рознится только часть Where. в остальном это один и тот же Linq. То есть можно написать все красивее, пошагово определив все состовляющие.

var query = MongoDBInstance.GetMongoDatabase.GetCollection<User>("UserInfo")
    .AsQueryable<User>();
if (!string.IsNullOrEmpty(searchByName)) {
    var upperCaseSearchString = searchByName.ToUpper()
    string[] names = upperCaseSearchString.Split(' ');
    if (names.Length > 1) {
         query = query.Where(i=>
                            !i.IsActivated && ((
                                i.Name.ToUpper().Contains(names[0]) &&
                                i.LastName.ToUpper().Contains(names[1])) ||
                            (i.Name.ToUpper().Contains(names[1]) &&
                             i.LastName.ToUpper().Contains(names[0]))));
    }
    else {
        query = query.Where(i=>
                            !i.IsActivated && (
                                i.Name.ToUpper().Contains(upperCaseSearchString) ||
                                i.LastName.ToUpper().Contains(upperCaseSearchString)));
          
    }
}
else {
    query = query.Where(i=>!i.IsActivated);
}
query = query.OrderBy(i=>i.ID);
if (query.Any()) {
    ViewData["uncheckedProfiles"] = query.ToPagedList(pageNumber, pageSize);
}
else {
    ViewData["uncheckedProfilesSearch"] = "NoResult";
}

За скобки не ручаюсь, писал прям в редакторе тостера

Comments

[Макс Максимов]

Это именно то, чему мне следует научиться. Спасибо!

[Алексей Кулаков]

Еще могу добавить что все методы из Linq унифицированы и работают с IEnumerable. и с ними нужно быть поосторожнее, потому как может случиться перебор всей коллекции несколько раз. А вообще есть в ReSharper функция просмотра исходных кодов. Decompile sorces в оциях найдите. и через Shift+Left mouse click можно открывать много всякого интересного для себя кода из библиотек microsoft-а

Answer 2

[Владислав Килин]

Могу ошибаться, но тем не менее, запрос с ToUpper в C# driver mongoDb выглядит не очень оптимальным. Увы, сейчас нет возможности проверить, но скорее всего для более быстрого решения этой задачи придется создать поле-индекс, в котором уже хранить имя и/или фамилию в upper case, для быстрого к ним доступа через регулярку.

Также многие разработчики на .NET MVC считают, что обращения к базе данных прямо из контроллера, равно как и вывод данных на вьюхе через ViewData - это моветон.

Я, увы, не могу написать вам сообщение на хабре, из-за вашего режима ридонли, однако же могу предложить вам поупражняться с подобными задачами на моем проекте, который как раз на ASP.NET MVC + mongoDb/MSSQL. Напишите в Skype v_qilin, если вам интересно.

Answer 3

[Vit]

Выносите часть кода в отдельные функции, чтобы не углублять сильно уровень вложенности.

Answer 4

[callback]

Много дублирования кода. Выносите блок, который повторяется 3 раза, в отдельную функцию, и вызывайте её с соответствующими параметрами