Skip to content

MOBILE - homeScreen and AddTaskScreen refactoring#106

Closed
CroyzCamel wants to merge 3 commits intodevelopfrom
feat/update-add-task-screen
Closed

MOBILE - homeScreen and AddTaskScreen refactoring#106
CroyzCamel wants to merge 3 commits intodevelopfrom
feat/update-add-task-screen

Conversation

@CroyzCamel
Copy link
Contributor

@CroyzCamel CroyzCamel commented Apr 5, 2025

modificação das respostas das atividades para a nova versão;

Issue

This pull request refers to Issue #

  • Does not apply to this pull request

Description

screen modification, adding layout for suitability

Types of changes

  • New feature (non-breaking change which adds functionality).
  • Bug fix (non-breaking change which fixes an issue).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).

What changed:

  • UI
  • Business rule
  • Documentation
  • Gradle or Build
  • Navigation
  • Tests
  • Resource files
  • Project architecture
  • Code style

Checklist

  • Have tested the changes.
  • Met all the acceptance requirements of this task.
  • Verified if branch is up-to-date with develop (if not - rebase it or merge it).
  • Resolve git conflicts

If any of the acceptance criteria are not met, please explain why below

What is the criteria? Why it is different?

modificação das respostas das atividades para a nova versão;
@CroyzCamel CroyzCamel self-assigned this Apr 5, 2025
@CroyzCamel CroyzCamel changed the title homeScreen and AddTaskScreen refactoring MOBILE - homeScreen and AddTaskScreen refactoring Apr 5, 2025
@CroyzCamel CroyzCamel removed the request for review from edijaniosouza April 5, 2025 23:31
OdisBy
OdisBy previously approved these changes Apr 7, 2025
Copy link
Collaborator

@OdisBy OdisBy left a comment

Choose a reason for hiding this comment

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

Boa, ficou bem legal assim ❤️

Coloquei alguns comentários mas alguns são dúvidas tbm, pode debater lá de boa!

Ah e se atente a colocar uma linha em branco no final do arquivo, o github costuma reclamar quando está faltando! Esse PR pode deixar sem msm, mas nos próximos vou cobrar rs.

val type: TaskType
) {
companion object {
private fun create(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pq está criando essa função? Pode usar o próprio construtor da Task, não?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bem diretamente daria certo também. Mas eu basicamente, tentei implmentar algo do design patterns chamado Factory Method.
https://johankovalsikoski.medium.com/kotlin-design-patterns-2-factory-method-4350487a8a#:~:text=O%20Factory%20Method%20é%20sobre,a%20partir%20de%20um%20parâmetro.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm eu entendo, esse padrão é dhr msm. No entanto acho que ele não se aplica tanto nesse caso de ter quase todos os paramêtros (ou todos) no Factory.

Um exemplo que seria interessante por exemplo seria se pudessemos salvar só com o título por exemplo, e termos valores default para os outros:

private fun create(
            title: String,
        ): Task {
            return Task(
                id = UUID.random(),
                title = title,
                description = "",
                category = Category.Default,
                date = LocalDate.now(),
                type = Type.Default,
                isSelected = false
            )
        }

nesse caso para criar o objeto passaria apenas o título 😄, há outros casos tbm, mas para recriar todo o construtor dificilmente acho a pena, a não ser que vá passar nomes mais claros por exemplo, que não poderia estar no model mas pode estar nesse factory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Mas pode manter desse jeito sem problemas, só queria saber o motivo msm, não tem problemas

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Show show,
Irei passar novamente para compreender melhor

@CroyzCamel CroyzCamel enabled auto-merge April 8, 2025 22:39
@CroyzCamel CroyzCamel requested a review from OdisBy April 8, 2025 22:50
@CroyzCamel
Copy link
Contributor Author

Ruliam, meu jovem. Essa mezera do github não me ajude a resolver esses conflitos

@OdisBy
Copy link
Collaborator

OdisBy commented Apr 10, 2025

Fechando como conversamos!

@OdisBy OdisBy closed this Apr 10, 2025
auto-merge was automatically disabled April 10, 2025 13:55

Pull request was closed

@CroyzCamel CroyzCamel linked an issue Apr 18, 2025 that may be closed by this pull request
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.

MOBILE - MODIFICAÇÃO DA LÓGICA ADICIONAR TASK

2 participants