Add Task.IsCompletedSuccessfully

[gkhanna79]

@GSPP commented on Sat Aug 13 2016

Task already has a few shortcut properties: IsFaulted, IsCancelled, IsCompleted. These are useful because they provide a way to write certain conditions very succinctly.

The only such property missing is IsCompletedSuccessfully. It should be added.

The workarounds are a little awkward: t.Status == TaskStatus.RanToCompletion and !t.IsFaulted && !t.IsCanceled. They are longer, don't express meaning well and sometimes are written incorrectly (e.g. forgetting about cancellation).

Is there a reason IsCompletedSuccessfully can/should not exist?

---

@omariom commented on Sat Aug 13 2016

IsSucceeded?

---

@benaadams commented on Sat Aug 13 2016

IsCompletedSuccessfully would match ValueTask which has it

https://github.com/dotnet/corefx/blob/master/src/System.Threading.Tasks.Extensions/src/System/Threading/Tasks/ValueTask.cs#L135-L136

---

@GSPP commented on Sat Aug 13 2016

Yes, I think Task and ValueTask should have as much common surface area as possible to make it easy to switch between the two.

It might even make sense to add a Status property to ValueTask and synthesize a suitable status in case the object is not based on a task.

---

@jamesqo commented on Sun Aug 14 2016

Since ValueTask has it, 👍 from me.

Also I thought API requests go in corefx? (at least that's what I got told here)

---

@GSPP commented on Mon Aug 15 2016

I believe mscorlib types are supposed to be discussed here. I always check the file system on Github first to see where a given type lives. @jamesqo

---

@omariom commented on Mon Aug 15 2016

IsCompletedSuccessfully would match ValueTask which has it

Is it late to change ValueTask? )

---

@ghost commented on Mon Aug 15 2016

IsSucceeded?

HasSucceeded?

just bikesheding .. :)

[benaadams]

Also t.Status == TaskStatus.RanToCompletion makes a non-inline call to get Status because Task.Status a bit of a chunky function.

/cc @stephentoub

[stephentoub]

I agree it's a good property to add.

[stephentoub]

Note, too, that the property already exists... it's just currently called IsRanToCompletion and is internal:
https://github.com/dotnet/coreclr/blob/master/src/mscorlib/src/System/Threading/Tasks/Task.cs#L1471
so the implementation work here is trivial.

[karelz]

Might be good candidate for easy up-for-grabs task once we review and approve the API.

[terrajobst]

We think

IsCompletedSuccessfully

makes the most sense:

• It's consistent to the other IsXxx method
• Makes it clear that success is a subcase of completion

[mgravell]

@terrajobst plus that makes it directly equivalent to ValueTask<T> (which should be updated to use that in the IsCompletetedSuccessfully property when available) :)