Real-Serious-Games / C-Sharp-Promise

Promises library for C# for management of asynchronous operations.
MIT License
1.2k stars 149 forks source link

Multiple async operations cascade #31

Closed nah0y closed 7 years ago

nah0y commented 7 years ago

Hello again! Sorry if I post a lot of issues that are just questions :/

I followed the ReadMe but can't find how to do something like this: Picture.GetListFromId and Tag.GetListFromPicture are both promises.

Picture.GetListFromId(3)
.Then(pictureList =>
{
    for (int i = 0; i < pictureList.Count; i++)
    {
        return Tag.GetListFromPicture(pictureList[i]);
    }
})
.Then(tagList =>
{
});

What I want to achieve, is first get the list of pictures, and then with this list, I want to get the tags for each picture. So if I have 3 pictures, each having 5 tags, it would:

I don't even know if this is possible...

Also in my example here, I would get a callback with the tagList, but I won't have the pictureList[i] variable that I passed (and I need it) :/

Thanks a lot!

adamsingle commented 7 years ago

This one comes down to the structure of your code I think. But first we need to understand a few things about what you're trying to do. I assume GetListFromId() is asynchronous, hence the promise. This gives you back a List or something similar? Then for each one of those you want to call GetListFromPicture()? (On a side note I would reconsider you method names. GetList is quite ambiguous). This is also asynchronous and returns a list of tags? in your pseudocode what is Tag? It's a singleton style class of all the tags and their associated pictures? What is it you want back from this function? a collection of lists of tags?

The crux of the matter here I think is that you want something like the following: PromiseFunction() { //do this and resolve, triggering the 'then' //now do it again and resolve, triggering the 'then' again //and do it again, resolve and trigger the 'then' again } .Then(x => { //this has been called three times });

You might be able to write a custom promise function that could do something like this and maybe that would be a handy thing to have in your programmers toolbox, but I can't see a reason I would ever need it personally and I believe it would break the A+ spec if we had it in the library. A promise doesn't move to the next promise in the chain until it has resolved, and once resolved, does not resolve again.

If I was doing this, I would have a promise wrapping this up that resolves once all three internal promises have completed and returns you a List<List>

If you also need the picture list, then you probably want a small class, maybe with a List and a List<List> or something like that and have your overall promise return that

nah0y commented 7 years ago

Hey, Thanks for answering!

Yes Picture.GetListFromId is asynchronous. This give back a list of pictures. And yes for each of these pictures, I want to call Tag.GetListfromPicture which is also asynchronous and return a list of tags, correct.

Tag is just a "model" class:

public class Tag
{
    public int id;
    public string data;
    public int pictureId;
    public int patientId;
}

Same thing for Picture:

public class Picture
{
    public int id;
    public DateTime createdAt;
    public int patientId;
    public string fileName;
    public bool hasOriginalFile;
}

About reconsidering method names... I think this is more than explicit. I mean, you call Picture.GetListFromPatient, which returns a list of pictures based on a patient id. (Sorry I wrote Picture.GetListFromId, but it's FromPatient in my code). And you have Tag.GetListFromPicture, which returns a list of tags based on a picture id. I don't see what's ambiguous about it :/ (but I'd love to know!)

So, each picture has some tags attached to it, and in my database, I have a separate table for tags, which have a pictureId to link to the picture.

And from what I understand about your answer, I should make a "higher" level Promise that does everything I want first internally first. That makes sense. But maybe it make more sense to modify my Picture class, to have a tag list, and the web server request would directly get all the tags in the database for each picture before returning the result. That would be better I think. Instead of having multiple web requests to make, I can just do that once and the server make all the database requests in one go.

Thanks a lot!

RoryDungan commented 7 years ago

If you want to run Tag.GetListFromPicture on each picture before triggering the final .Then, I think you want something like this:

Picture.GetListFromId(3)
.Then(pictureList =>
{
    return Promise.All(pictureList.Select(picture => Tag.GetListFromPicture(picture));
})
.Then(tagList =>
{
});

I can't guarantee that code will exactly suit your needs, so also check out the section in the docs on combining multiple async operations because I think that's what you're trying to do.

nah0y commented 7 years ago

Hey Rory, Thanks for the answer!

Yes I was trying to combine multiple async operations. I tried what you propose (modifying it a bit to fix things):

PictureClient.GetListFromPatient(3)
.Then(pictureList =>
{
    return Promise.All(pictureList.Select(picture => TagClient.GetListFromPicture(picture.id)).ToArray());
})
.Then(tagList =>
{
});

The thing is that it has 2 errors:

PS: I found another way to do that, so if you don't want to lose time with that, that's totally fine :D

RoryDungan commented 7 years ago

Hey, sorry it's taken me so long to get back to you. Just change that Promsie.All to Promise<List<Picture>>.All to tell it that you want to use that type of promise. Replace List<Tag> with the actual type that's returned by TagClient.GetListFromPicture though, because I'm just assuming it returns a list of tags.

The reason you need to do this is because generic promises (i.e. promises that return a value) are separate from non-generic promises, and just using Promise.All on its own implies that you're using the non-generic version where actually you wanted to use the generic one. Sorry my answer before had that mistake in it.

nah0y commented 7 years ago

Hey,

Thanks for the answer, no problem. This make total sense, thanks for the perfect explanation!