Should we get rid of ParseResponse?

[fischerscode]

From a first look, this might sound ridiculous, but from what I have seen, ParseResponse is always used to provide either a object or an error. This use case almost shouts "Future" :-) In addition to this, ParseRespons is almost always wrapped in a Future.

The advantages of removing ParseRespons would be type safety and from my perspective simpler interaction with the SDK (don't check if ParseRespons contains an error, simply use try-catch).

This is definitely a breaking change. But I think it would be worth it.

[phillwiggins]

This is definitley something that needs to be considered quite heavily. I'm not sure if it's the Android developer in me that seems attached to Response wrapper classes vs objects but I do like the idea of Response classes. The idea behind them is that they implement all the try catch logic for you so that you don't have to.

Would this allow a stronger type safety? I think the Response class would be sufficient if the generics was mastered correctly?

[fischerscode]

But isn't Future the Response wrapper already?

Of course we could achieve type safety with generics, too. But as the Response class has the field result as well as results, I think this could be quite hard. As result could be either List<T> or T.

I just think, it might make the code cleaner and supporting chaining actions like this return await someFunctionThatChangesSomething(await queryBuilder.first()).save() or print(await queryBuilder.count()would be quite nice.

[phillwiggins]

Yes, result should be deprecated in all honest. It should always be results that we look at. Although now you have said that it might be better just using a Future? It's a fairly large change though.

[fischerscode]

I might start implementing some parts. Then we can have a look if it's worth it.

[phillwiggins]

Yep, more than happy to have a look at this. As I say, I think I wrote from an Android developers prospective where this is a common pattern.

On Fri, 4 Sep 2020, 14:14 Maximilian Fischer, notifications@github.com wrote:

I might start implementing some parts. Then we can have a look if it's worth it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/parse-community/Parse-SDK-Flutter/issues/437#issuecomment-687137248, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4CPXV3NO7IMFJN56DZQGLSEDR37ANCNFSM4QWOTIKQ .

[nstrelow]

I should check the issues more frequently. I would actually love that change. I implemented some form of FutureBuilder, which displays a retry button on fail.

For ParseResponse I had to unwrap it first, to get a potential error.

So from my side this was a pain point and I would love the greater type safety.

[fischerscode]

I've found no time implementing this in a test-branch, but I will give this a try in the future. In many cases using extensions helps to reduce unwrapping.

import 'package:parse_server_sdk_flutter/parse_server_sdk.dart';

extension ExtendedQueryBuilder<T extends ParseObject> on QueryBuilder<T> {
  Future<T> first() async {
    setLimit(1);
    ParseResponse parseResponse = await query();
    if (parseResponse.error != null) {
      throw parseResponse.error;
    } else {
      return parseResponse.results.first;
    }
  }

  Future<List<T>> find() async {
    ParseResponse parseResponse = await query();
    if (parseResponse.error != null) {
      throw parseResponse.error;
    } else {
      return parseResponse.results?.map<T>((e) => e)?.toList() ?? [];
    }
  }
}

[nstrelow]

Chiming in about results ParseConfig returns only result and not results.

Just to keep that in mind, when refactoring.

[NadjibR]

I've found no time implementing this in a test-branch, but I will give this a try in the future. In many cases using extensions helps to reduce unwrapping.

import 'package:parse_server_sdk_flutter/parse_server_sdk.dart';

extension ExtendedQueryBuilder<T extends ParseObject> on QueryBuilder<T> {
  Future<T> first() async {
    setLimit(1);
    ParseResponse parseResponse = await query();
    if (parseResponse.error != null) {
      throw parseResponse.error;
    } else {
      return parseResponse.results.first;
    }
  }

  Future<List<T>> find() async {
    ParseResponse parseResponse = await query();
    if (parseResponse.error != null) {
      throw parseResponse.error;
    } else {
      return parseResponse.results?.map<T>((e) => e)?.toList() ?? [];
    }
  }
}

This saves my day.

I was trying to make ParseResponse works with ListView.Builder inside FutureBuilder with no chance.

[gunhaxxor]

I personally second getting rid of ParseResponse. I don't see the purpose of it. I almost always end up creating a future from my queries or save-calls. Like this:

Future<ParseObject> createThing() async{
  ParseObject thing = ParseObject("Thing")
  ParseResponse response = await thing.save();
  if(response.success){
    return response.results.first;
  }
  return Future.error(response.error);
}

And the save-function and queryBuilder returns a future in the first place. So my understanding is that the above code is doing the following.

• call save and wait for it's future
• the future aalways (?) resolves without an error (despite whether the operation was actually successful or not)
• inspect the resolved response to see if it actually was successful.
• If it was, return/resolve with the data . If it wasn't, create and return an error future.

To me this seems like a lot of boilerplate.

[2shrestha22]

Error handling is troublesome, we need a better way.

[Mabenan]

In my opinion the client dev always knows or at least expects if it is a list or a single entity. Thats why all request methods should return Future or Future<List>.

If we come to the conclusion that we cannot remove Response. Can we at least change it to support Generic so that we have a little bit more Typesaftey at the dev time.