Skip to content

Fix sorting completed#145

Merged
amikhaylin merged 1 commit intomasterfrom
completed-sorting
Sep 4, 2025
Merged

Fix sorting completed#145
amikhaylin merged 1 commit intomasterfrom
completed-sorting

Conversation

@amikhaylin
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review by ChatGPT

innerTasks = tasks.sorted(by: TasksQuery.sortingWithCompleted)
}

if searchText.isEmpty {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ваш код демонстрирует позитивные изменения, направленные на упрощение сортировки задач. Однако есть несколько моментов, которые стоит рассмотреть для улучшения:

  1. Проверка эквивалентности сортировок:

    • Ранее вы выполняли двойную сортировку: сначала TasksQuery.defaultSorting, затем TasksQuery.sortCompleted. Убедитесь, что TasksQuery.sortingWithCompleted правильно комбинирует эти два критерия сортировки и выдает ожидаемый порядок задач. Возможно, стоит написать юнит-тесты для проверки этого поведения.
  2. Оптимизация сортировки:

    • Использование одной сортировки вместо двух может повысить производительность, что является положительным моментом. Однако убедитесь, что новая сортировка не нарушает логику отображения задач в различных списках.
  3. Улучшение читаемости фильтрации в .today:

    • Условие фильтрации в кейсе .today выглядит несколько сложным:
      tasks.filter({ TasksQuery.checkToday(date: $0.completionDate) && ($0.completed == false || ($0.completed && $0.parentTask == nil)) })
      Рассмотрите возможность разделения этого условия на несколько строк или использования вспомогательных методов для повышения читаемости:
      tasks.filter { task in
          TasksQuery.checkToday(date: task.completionDate) &&
          (!task.completed || (task.completed && task.parentTask == nil))
      }
  4. Единообразие именования:

    • sortingWithCompleted – название понятно, но можно рассмотреть альтернативы для большей ясности, например, sortedByCompletionStatus или combinedSorting.
  5. Повторное использование кода:

    • Поскольку сортировка TasksQuery.sortingWithCompleted используется в нескольких кейсах, убедитесь, что это не приводит к дублированию логики. Возможно, стоит вынести сортировку за пределы switch, если это применимо для всех кейсов.
  6. Документация и комментарии:

    • Добавьте комментарии к новым методам сортировки, объясняющие, как они работают и почему были внесены изменения. Это поможет другим разработчикам быстрее понять логику изменений.
  7. Обработка исключений:

    • Убедитесь, что методы сортировки и фильтрации устойчивы к неожиданным значениям данных, таким как nil или некорректные даты.

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

CURRENT_PROJECT_VERSION = 65;
DEAD_CODE_STRIPPING = YES;
GENERATE_INFOPLIST_FILE = YES;
MACOSX_DEPLOYMENT_TARGET = 14.0;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вы проделали работу по обновлению CURRENT_PROJECT_VERSION с 64 на 65 во множественных конфигурациях сборки. Однако есть несколько аспектов, которые стоит учесть для повышения качества и устойчивости проекта:

  1. Централизация управления версией:

    • Проблема: Обновление версии вручную во множестве мест может привести к несоответствиям и ошибкам, если какое-то из мест будет пропущено.
    • Решение: Рассмотрите возможность использования xcconfig файлов для хранения общих настроек сборки, включая номер версии. Это позволит задавать CURRENT_PROJECT_VERSION в одном месте и избежать дублирования.
  2. Автоматизация повышения версии:

    • Проблема: Ручное повышение версии может быть подвержено ошибкам и требует дополнительного времени.
    • Решение: Используйте скрипты или инструменты, такие как agvtool, для автоматизации процесса повышения номера версии. Это обеспечит последовательность и снизит вероятность человеческих ошибок.
  3. Проверка совместимости версий:

    • Проблема: Обновление номера версии может потребовать дополнительных изменений в зависимости от версии, например, в MARKETING_VERSION.
    • Решение: Убедитесь, что все связанные параметры, такие как MARKETING_VERSION, также обновлены при необходимости и соответствуют новым требованиям приложения.
  4. Документация изменений:

    • Проблема: При множественных изменениях в конфигурациях сборки может быть сложно отслеживать причины и контекст изменений.
    • Решение: Добавьте комментарии в файлы конфигурации или в пулл-реквест, объясняющие причину повышения версии. Это облегчит понимание изменений для других участников команды.
  5. Проверка влияния на CI/CD:

    • Проблема: Обновление версии может повлиять на процессы непрерывной интеграции и доставки.
    • Решение: Проверьте, как изменения CURRENT_PROJECT_VERSION влияют на ваши CI/CD пайплайны. Убедитесь, что все автоматические процессы корректно обрабатывают новую версию.
  6. Consistency across Targets and Extensions:

    • Проблема: Некоторые таргеты или расширения могут использовать разные настройки версии, что может вызвать несоответствия.
    • Решение: Проверьте, что все таргеты и расширения проекта согласованы в использовании номера версии. Возможно, стоит использовать общие настройки или параметры, чтобы сохранить консистентность.
  7. Использование Семантического Версионирования:

    • Проблема: Простое увеличение номера версии может не отражать изменения в функциональности или исправлениях.
    • Решение: Рассмотрите возможность применения семантического версионирования, где версия состоит из мажорной.минорной.патч (например, 1.2.3). Это даст более четкое представление о типе изменений в каждой версии.

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

.sorted(by: TasksQuery.sortingWithCompleted),
id: \.self) { task in
if let subtasks = task.subtasks, subtasks.count > 0 {
OutlineGroup([task],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Внесённое изменение заменяет метод сортировки с TasksQuery.defaultSorting на TasksQuery.sortingWithCompleted. Вот несколько замечаний и рекомендаций по этому изменению:

  1. Проверка реализации sortingWithCompleted:

    • Убедитесь, что метод sortingWithCompleted корректно обрабатывает все возможные состояния задач, включая завершённые и незавершённые.
    • Проверьте, что этот метод стабильно возвращает ожидаемый порядок задач и не вызывает непредвиденных побочных эффектов.
  2. Влияние на производительность:

    • Если sortingWithCompleted использует более сложную логику сортировки по сравнению с defaultSorting, убедитесь, что это не негативно сказывается на производительности, особенно при большом объёме данных.
  3. Уникальность id:

    • В текущем коде используется id: \.self, что предполагает, что элементы уникальны сами по себе. Если структура задачи (task) не подтверждает Hashable или если возможны дубликаты, рассмотрите использование уникального идентификатора, например id: \.id.
  4. Тестирование:

    • Проведите тщательное тестирование интерфейса после изменения сортировки, чтобы убедиться, что порядок отображения задач соответствует ожиданиям.
    • Проверьте случаи, когда задачи имеют одинаковые значения по полям, используемым в сортировке, чтобы убедиться в стабильности порядка.
  5. Документация:

    • Если метод sortingWithCompleted добавляет новую логику, рекомендуется обновить документацию или добавить комментарии, объясняющие, как и почему используется эта сортировка.
  6. Consistency:

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

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

ForEach(section == .completed ? searchResults.filter({ $0.completed && $0.parentTask == nil }) : searchResults.filter({ $0.completed == false }),
id: \.self) { task in
if let subtasks = task.subtasks, subtasks.count > 0 {
OutlineGroup([task],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ваши изменения демонстрируют стремление улучшить сортировку задач, что положительно. Однако есть несколько моментов, на которые стоит обратить внимание для повышения качества кода:

  1. Повторный вызов project.getTasks():
    Каждый раз при доступе к searchResults вызывается project.getTasks(). Если этот метод выполняет ресурсоемкие операции (например, обращение к базе данных или сети), это может негативно сказаться на производительности. Рассмотрите возможность кэширования результата или использования механизма @State/@Published для хранения задач.

  2. Последовательность сортировки и фильтрации:
    В текущем виде вы сначала фильтруете, а затем сортируете. Это обычно оптимально, но убедитесь, что TasksQuery.sortingWithCompleted действительно учитывает все необходимые критерии сортировки после фильтрации. Если сортировка зависит от исходного порядка списка, возможны непредвиденные результаты.

  3. Изменение сортировки в ForEach:
    В одном из изменений вы заменили TasksQuery.defaultSorting на TasksQuery.sortingWithCompleted, что может привести к несогласованности сортировки в разных частях интерфейса. Убедитесь, что новая сортировка соответствует общим требованиям дизайна и пользователю будет интуитивно понятна.

  4. Удаление сортировки в последнем ForEach:
    В последнем изменении вы убрали сортировку для секции завершенных и незавершенных задач. Это может привести к тому, что задачи в этих секциях будут отображаться в произвольном порядке, что ухудшит пользовательский опыт. Рекомендуется либо вернуть сортировку, либо обеспечить другую форму упорядочивания для ясности.

  5. Использование id: \.self:
    Использование \.self в качестве идентификатора подходит, если объекты Todo соответствуют Hashable и уникальны. Однако, если есть риск дублирования или изменения свойств, влияющих на хэш, лучше использовать уникальный идентификатор, например, \.id, если он доступен.

  6. Оптимизация фильтрации:
    В нескольких местах вы фильтруете searchResults по parentTask == nil или другим критериям. Если фильтрация и сортировка выполняются часто, рассмотрите возможность использования более эффективных структур данных или предварительной обработки данных для минимизации вычислительных затрат.

  7. Улучшение читаемости кода:
    Чтобы повысить читаемость и поддерживаемость, можно извлечь сложные фильтры и сортировки в отдельные методы или расширения. Это облегчит понимание логики и позволит переиспользовать код в будущем.

Пример улучшенного подхода:

struct ProjectTasksListView: View {
    // ...

    var sortedTasks: [Todo] {
        project.getTasks().sorted(by: TasksQuery.sortingWithCompleted)
    }

    var searchResults: [Todo] {
        let tasks = searchText.isEmpty ? sortedTasks : sortedTasks.filter {
            $0.name.localizedStandardContains(searchText)
        }
        return tasks
    }

    // Остальной код...
}

Это уменьшает количество вызовов getTasks() и упрощает логику фильтрации и сортировки.

return tasks.filter { $0.name.localizedStandardContains(searchText) }.sorted(by: TasksQuery.sortingWithCompleted)
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Безопасное распаковка опционала:

    В текущей реализации вы используете принудительное разворачивание mainTask.subtasks!, что может привести к падению приложения, если subtasks вдруг окажется nil несмотря на предварительную проверку. Вместо этого рекомендуется использовать оператор ?? для предоставления пустого массива по умолчанию:

    let tasks: [Todo] = mainTask.subtasks ?? []
  2. Оптимизация сортировки:

    Если сортировка TasksQuery.sortingWithCompleted используется в обоих случаях (с пустым searchText и с фильтрацией), то повторяющийся код можно избежать. Возможно, стоит вынести сортировку за пределы условия:

    let filteredTasks = searchText.isEmpty
        ? tasks
        : tasks.filter { $0.name.localizedStandardContains(searchText) }
    
    return filteredTasks.sorted(by: TasksQuery.sortingWithCompleted)

    Это повысит читаемость и уменьшит дублирование кода.

  3. Проверка логики сортировки:

    Убедитесь, что метод TasksQuery.sortingWithCompleted корректно обрабатывает все возможные состояния задач, включая завершенные и незавершенные. Возможно, стоит добавить тесты, чтобы гарантировать правильность сортировки в разных сценариях.

  4. Использование guard для раннего выхода:

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

  5. Документация и комментарии:

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

  6. Оптимизация поиска:

    Если массив tasks может быть большим, рассмотрите возможность оптимизации поиска, например, используя более эффективные методы поиска или предварительную индексацию данных.

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

innerTasks = tasks.sorted(by: TasksQuery.sortingWithCompleted)
}

if searchText.isEmpty {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Повторение кода:
    Вы повторяете метод tasks.sorted(by: TasksQuery.sortingWithCompleted) во всех случаях, кроме .today, где дополнительно применяете фильтр. Это нарушает принцип DRY (Don't Repeat Yourself). Рекомендуется вынести общую часть сортировки за пределы switch или использовать более компактную структуру.

    let filteredTasks: [Todo]
    switch list {
    case .inbox:
        filteredTasks = tasks
    case .today:
        filteredTasks = tasks.filter { TasksQuery.checkToday(date: $0.completionDate) }
    case .tomorrow:
        filteredTasks = tasks
    case .alltasks:
        filteredTasks = tasks
    default:
        filteredTasks = tasks
    }
    
    let innerTasks = filteredTasks.sorted(by: TasksQuery.sortingWithCompleted)
  2. Использование default кейса:
    Если вы обрабатываете все возможные случаи перечисления list, возможно, default кейс не нужен. Это улучшит безопасность кода, так как при добавлении новых кейсов в перечисление компилятор будет сигнализировать о необходимости обновления switch.

  3. Название метода сортировки:
    Название sortingWithCompleted может быть не совсем понятным. Лучше использовать более описательное название, например, sortedByCompletionStatus или sortByCompletedTasks, чтобы сразу было ясно, что именно делает метод сортировки.

  4. Оптимизация фильтрации:
    Если фильтрация для .today является единственным случаем, где она необходима, убедитесь, что метод TasksQuery.checkToday эффективно реализован, чтобы избежать потенциальных проблем с производительностью при большом количестве задач.

  5. Дополнительные улучшения SwiftUI:
    Если в дальнейшем планируется расширение функциональности списка задач, рассмотрите использование @ViewBuilder или отдельных подкомпонентов для разных типов списков. Это поможет разделить логику и представление, сделав код более модульным.

Пример возможной рефакторинга:

struct TasksListView: View {
    var body: some View {
        let innerTasks = getSortedTasks()
        
        if searchText.isEmpty {
            // Ваши дальнейшие действия
        }
    }
    
    private func getSortedTasks() -> [Todo] {
        let filteredTasks: [Todo]
        switch list {
        case .inbox, .tomorrow, .alltasks:
            filteredTasks = tasks
        case .today:
            filteredTasks = tasks.filter { TasksQuery.checkToday(date: $0.completionDate) }
        }
        return filteredTasks.sorted(by: TasksQuery.sortingWithCompleted)
    }
}

Этот рефакторинг уменьшает повторение кода и делает логику более ясной.

static func deleteTask(context: ModelContext, task: Todo) {
task.disconnectFromAll()
task.deleteSubtasks(context: context)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Безопасное развертывание опционалов:

    • В строке first.dueDate! < second.dueDate! вы используете принудительное развертывание (!). Это может привести к аварийному завершению приложения, если dueDate окажется nil. Рекомендуется использовать безопасное развертывание:
      if let firstDueDate = first.dueDate, let secondDueDate = second.dueDate {
          return firstDueDate < secondDueDate
      } else {
          // Обработка случая, когда одна из дат отсутствует
      }
  2. Упрощение логики сравнения завершенных задач:

    • Ваши текущие условия для проверки completed можно упростить, используя логические операторы более эффективно. Например:
      if first.completed != second.completed {
          return !first.completed
      }
      Это сокращает код и делает его более читаемым.
  3. Обработка дат завершения:

    • В вашем измененном коде вы добавили проверку на наличие completionDate. Это хорошее дополнение, однако текущая логика возвращает true если firstCompletionDate >= secondCompletionDate. Возможно, более интуитивно логично сортировать по возрастанию дат завершения:
      return firstCompletionDate < secondCompletionDate
      Или, если вы хотите сортировать по убыванию:
      return firstCompletionDate > secondCompletionDate
  4. Обработка случая, когда одна дата завершения отсутствует:

    • В текущей реализации, если одна из completionDate равна nil, будет выполнен возврат (first.completed && second.completed). Возможно, имеет смысл определить, как именно вы хотите сортировать задачи, у которых отсутствует completionDate. Например:
      if first.completed != second.completed {
          return first.completed
      }
      if let firstCompletionDate = first.completionDate, let secondCompletionDate = second.completionDate {
          return firstCompletionDate < secondCompletionDate
      }
      return false // или true, в зависимости от требуемой логики
  5. Форматирование и стилистика:

    • Обратите внимание на лишние пробелы и пустые строки, которые были добавлены. Поддержание единообразного стиля кода улучшает его читаемость:
      if first.completed == false && second.completed {
          return true
      }
      if first.completed && second.completed == false {
          return false
      }
      
      if let firstCompletionDate = first.completionDate, let secondCompletionDate = second.completionDate {
          return firstCompletionDate >= secondCompletionDate
      }
      
      return first.completed && second.completed
  6. Дополнительные улучшения:

    • Рассмотрите возможность использования сортировки с использованием sorted(by:), где вы можете определить более сложную логику сортировки в одном месте, повысив читаемость:
      tasks.sorted { (first, second) -> Bool in
          // Ваша логика сортировки
      }

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

@amikhaylin amikhaylin merged commit 10eec06 into master Sep 4, 2025
1 check passed
@amikhaylin amikhaylin deleted the completed-sorting branch September 4, 2025 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant