Skip to content

Conversation

@thsamajiki
Copy link
Collaborator

Kakao, Google 로그인 기능 구현했습니다.

@kailloop
Copy link
Collaborator

kailloop commented Jun 14, 2023

질문 및 건의사항이 있습니다!

  1. Log.i, Log.e 이런 방식으로 사용하신게 많으신데 DEBUG, INFO, ERROR 메소드 따로 사용 부탁드립니다!
  2. 자동로그인을 Splash에서 구현 하셨는데 특별히 SplashActivity에서 구현하신 이유가 있을까요?
  3. 자동로그인에 대한 명확한 기준이 필요해 보입니다. 해당 로직 플로우로 진행을 하게 된다면, 누가 Kakao Login을 해놓았던건지 Google Login을 해놓았던건지에 대한 분기처리부터 시작하여, 해당 자동로그인을 진행하는 로직에 데이터를 어떻게 저장할 것인지에 대한 부분까지 생각해보는게 어떨까요?

Copy link
Collaborator

@kailloop kailloop left a comment

Choose a reason for hiding this comment

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

setOnClickListeners는 따로 안쓰셔도 됩니다 :) BaseActivity에 있는걸 써주세요 ~!

Copy link
Collaborator

@kailloop kailloop left a comment

Choose a reason for hiding this comment

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

MainActivity의 getIntent 메소드 추가
getIntent(context: Context) = Intent(context, MainActivity::class.java) 해당 코드는 왜 쓰신걸까용..?

@thsamajiki
Copy link
Collaborator Author

thsamajiki commented Jun 14, 2023

@kailloop

  1. Log는 용도에 따라 수정하겠습니다.
  2. 자동 로그인은 SplashActivity에서 LoginActivity를 거치지 않고 바로 MainActivity로 이동해야 하기 때문에
    SplashActivity에서 자동 로그인을 구현해야 합니다. 이는 형구님도 그렇게 얘기하셨습니다.
  3. setOnClickListener는 수정하겠습니다.
  4. 팩토리 패턴을 사용해서 MainActivity로 이동할수 있도록 구현한 것입니다. 호출될 때마다 새 Intent 객체를 만들 필요가 없으니까요!

@thsamajiki thsamajiki requested a review from kailloop June 14, 2023 10:12
Copy link
Collaborator

@kailloop kailloop left a comment

Choose a reason for hiding this comment

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

팩토리 패턴에 대한 추가 질의사항이 있습니다!

  1. 팩토리 패턴 사용 이유가 수정 사항에 대한 제한을 열린 확장성을 가진 OCP 원칙을 지키기 위해서인데 해당 사항은 팩토리 패턴이라기 보다는 Singleton 패턴의 장점을 살리기위해 작성하신걸로 보입니다! 일단 의도는 그래 보여서 혹시 제가 아는 팩토리 패턴이랑 다른 것 같아 조심스레 여쭤봅니다..!
  2. 해당 메소드를 보시면 함수를 실행 시킬 때 마다 new Intent()를 생성해서 return 해주는 방식으로 저는 보입니다..! 그래서, 차라리
    companion object { val mainIntent : Intent? = null fun getIntent(): Intent { return if (mainIntent != null) mainIntent else mainIntent = Intent(this, MainActivity:class) } }
    해당 코드로 바뀌는게 좋지 않을까요?

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.

3 participants