parse-community / Parse-SDK-Flutter

The Dart/Flutter SDK for Parse Platform
https://parseplatform.org
Apache License 2.0
575 stars 188 forks source link

Question in the example/.../repository_user.dart #182

Closed jiangyang5157 closed 5 years ago

jiangyang5157 commented 5 years ago

Hi there, May I ask a few questions,

[master branch] flutter_parse_sdk/example/lib/data/repositories/user/repository_user.dart

@override
  Future<User> createUser(
      String username, String password, String emailAddress) async {
    api.createUser(username, password, emailAddress);

    final User user = await api.createUser(username, password, emailAddress);
    if (user != null) {
      await db.createUser(username, password, emailAddress);
    }

    return user;
  }

Q: Why do api.createUser(username, password, emailAddress); twice?

[master] flutter_parse_sdk/example/lib/data/repositories/user/repository_user.dart

@override
  Future<User> currentUser() => db.currentUser();

Q2: Why not return api.currentUser(), since db.currentUser() alway null?

phillwiggins commented 5 years ago

Hey Yang,

The first line is a mistake as it was a quick example of a repository pattern. There is no reason to call create user twice.

The second issue I'm not so sure on. I'm away from my desk but, why would db.currentUser always be null?

On Sat, Jun 1, 2019, 08:11 Yang JIANG notifications@github.com wrote:

Hi there, May I ask a few questions,

[master branch] flutter_parse_sdk/example/lib/data/repositories/user/repository_user.dart

@override Future createUser( String username, String password, String emailAddress) async { api.createUser(username, password, emailAddress);

final User user = await api.createUser(username, password, emailAddress);
if (user != null) {
  await db.createUser(username, password, emailAddress);
}

return user;

}

Q: Why do api.createUser(username, password, emailAddress); twice?

[master] flutter_parse_sdk/example/lib/data/repositories/user/repository_user.dart

@override Future currentUser() => db.currentUser();

Q2: Why not return api.currentUser(), since db.currentUser() alway null?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/phillwiggins/flutter_parse_sdk/issues/182?email_source=notifications&email_token=AB4CPXVBW7IJFI3546BB423PYIOLLA5CNFSM4HR74POKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GXCRZJQ, or mute the thread https://github.com/notifications/unsubscribe-auth/AB4CPXW4KOTA5B2YC22EQNTPYIOLLANCNFSM4HR74POA .

jiangyang5157 commented 5 years ago

Hi Phillwiggins,

Cool,, thank for the quick response to my question.

Reason of db.currentUser() is null is that the implementation returns null:

class UserProviderDB implements UserProviderContract {
  @override
  Future<User> currentUser() {
    return null;
  }
...
}

However, api.currentUser() is returning ParseUser::currentUser from SharedPreferences

class UserProviderApi implements UserProviderContract {
  @override
  Future<PUser> currentUser() {
    return ParseUser.currentUser();
  }
...
}

I see the idea of let database involve in **Repository is seems to provide a convenient way to access data in offline (without extra network request) for whatever reason (eg: diet daily plan for a specific user). Therefore, in the UserRepository case, we only need to handle create/destroy/save scenarios in db. More importantly, we only take action on db when api success.

For example when app want remove a user from server locally and online:

class UserRepository implements UserProviderContract {
  @override
  Future<ApiResponse> destroy(PUser user) async {
    ApiResponse response = await api.destroy(user);
    response = await db.destroy(user);
    return response;
  }
...
}

Should be:

ApiResponse response = await api.destroy(user);
if (response.success) {
   await db.destroy(user);
}
return response;

I just saw the sdk few days ago, please kindly correct me if I'm I get the idea wrong..

phillwiggins commented 5 years ago

No, you are correct. That logic would be more sound.

The repository was just a quick guide to help people with an offline first approach. There is a few bugs with it as it was added as an example. If you free, would you be able to do a pull request? I'd appreciate the help!

On Sat, Jun 1, 2019, 10:46 Yang JIANG notifications@github.com wrote:

Hi Phillwiggins,

Cool,, thank for the quick response to my question.

Reason of db.currentUser() is null is that the implementation returns null:

class UserProviderDB implements UserProviderContract { @override Future currentUser() { return null; } ... }

However, api.currentUser() is returning ParseUser::currentUser from SharedPreferences

class UserProviderApi implements UserProviderContract { @override Future currentUser() { return ParseUser.currentUser(); } ... }

I see the idea of let database involve in *Repository is seems to provide a convenient way to access data in offline (without extra network request) for whatever reason (eg: diet daily plan for a specific user). Therefore, in the UserRepository case, we only need to handle create/ destroy/save scenarios in db. More importantly*, we only take action on db when api success.

For example when app want remove a user from server locally and online:

class UserRepository implements UserProviderContract { @override Future destroy(PUser user) async { ApiResponse response = await api.destroy(user); response = await db.destroy(user); return response; } ... }

Should be:

ApiResponse response = await api.destroy(user);if (response.success) { response = await db.destroy(user); }return response;

I just saw the sdk few days ago, please kindly correct me if I'm I get the idea wrong..

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/phillwiggins/flutter_parse_sdk/issues/182?email_source=notifications&email_token=AB4CPXVV6EQYZQKMUXXBBXTPYJARBA5CNFSM4HR74POKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWW47SI#issuecomment-497930185, or mute the thread https://github.com/notifications/unsubscribe-auth/AB4CPXVNWQ3XMA2CVSMKCRLPYJARBANCNFSM4HR74POA .

jiangyang5157 commented 5 years ago

Thank you for the answer, Phill. Surething, probably in the nearly future after I get more understanding of how this sdk manage Parse. I would like to re-visit this example to see what I can do. At this moment, I'm still in the stage to make everything works as I expected.