Skip to content

Add or else methods to option closes #31 #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ax0l0tl merged 1 commit into from
Mar 10, 2026
Merged

Add or else methods to option closes #31 #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 34 additions & 3 deletions Source/FunicularSwitch/Option.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public Task<Option<T1>> Map<T1>(Func<T, Task<T1>> map) => Match(
public Option<T1> Bind<T1>(Func<T, Option<T1>> map) => Match(map, Option<T1>.None);

public Task<Option<T1>> Bind<T1>(Func<T, Task<Option<T1>>> bind) => Match(bind, () => Option<T1>.None);

public Option<T> OrElse(Option<T> other) => _isSome ? this : other;
public Option<T> OrElse(Func<Option<T>> other) => _isSome ? this : other();
public Task<Option<T>> OrElse(Task<Option<T>> other) => OrElse(() => other);
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

OrElse(Task<Option<T>> other) can leave other running without being awaited when the current option is Some. If other faults, the exception can become unobserved (and may surface via TaskScheduler.UnobservedTaskException). Consider removing this overload in favor of OrElse(Func<Task<Option<T>>>), or ensure exceptions from other are observed even when it isn't used (e.g., via a continuation).

Suggested change
public Task<Option<T>> OrElse(Task<Option<T>> other) => OrElse(() => other);
public Task<Option<T>> OrElse(Task<Option<T>> other)
{
other.ContinueWith(
t =>
{
var _ = t.Exception;
},
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously);
return OrElse(() => other);
}

Copilot uses AI. Check for mistakes.
public async Task<Option<T>> OrElse(Func<Task<Option<T>>> other) => _isSome ? this : await other().ConfigureAwait(false);

public void Match(Action<T> some, Action? none = null)
{
Expand Down Expand Up @@ -90,6 +95,8 @@ public async Task<TResult> Match<TResult>(Func<T, Task<TResult>> some, Func<Task

return await none().ConfigureAwait(false);
}


Comment on lines +98 to +99
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

There are several whitespace-only changes here (extra blank lines and trailing spaces). Please remove trailing whitespace and avoid blank lines containing indentation to keep diffs clean and match typical formatting conventions.

Suggested change

Copilot uses AI. Check for mistakes.

public async Task<TResult> Match<TResult>(Func<T, Task<TResult>> some, Func<TResult> none)
{
Expand Down Expand Up @@ -202,23 +209,47 @@ public static async Task<Option<TOut>> Bind<T, TOut>(this Task<Option<T>> bind,
var result = await bind.ConfigureAwait(false);
return await result.Bind(convert).ConfigureAwait(false);
}

public static async Task<Option<T>> OrElse<T>(this Task<Option<T>> option, Option<T> other)
{
var result = await option.ConfigureAwait(false);
return result.OrElse(other);
}

public static async Task<Option<T>> OrElse<T>(this Task<Option<T>> option, Func<Option<T>> other)
{
var result = await option.ConfigureAwait(false);
return result.OrElse(other);
}

public static async Task<Option<T>> OrElse<T>(this Task<Option<T>> option, Task<Option<T>> other)
{
var result = await option.ConfigureAwait(false);
return await result.OrElse(other).ConfigureAwait(false);
Comment on lines +225 to +228
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The Task<Option<T>> extension overload OrElse(..., Task<Option<T>> other) has the same unobserved-exception risk as the instance overload: if the awaited option is Some, other is never awaited, so faults may go unobserved. Prefer a Func<Task<Option<T>>> overload only, or observe other's exceptions when it isn't needed.

Suggested change
public static async Task<Option<T>> OrElse<T>(this Task<Option<T>> option, Task<Option<T>> other)
{
var result = await option.ConfigureAwait(false);
return await result.OrElse(other).ConfigureAwait(false);
public static Task<Option<T>> OrElse<T>(this Task<Option<T>> option, Task<Option<T>> other)
{
if (other == null) throw new ArgumentNullException(nameof(other));
// Ensure any exception on 'other' is observed even if 'option' is Some and short-circuits.
_ = other.ContinueWith(
t =>
{
var _ = t.Exception;
},
TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously);
// Delegate to the Func<Task<Option<T>>> overload to preserve existing behavior.
return option.OrElse(() => other);

Copilot uses AI. Check for mistakes.
}

public static async Task<Option<T>> OrElse<T>(this Task<Option<T>> option, Func<Task<Option<T>>> other)
{
var result = await option.ConfigureAwait(false);
return await result.OrElse(other).ConfigureAwait(false);
}

public static IEnumerable<TOut> Choose<T, TOut>(this IEnumerable<T> items, Func<T, Option<TOut>> choose) =>
items.SelectMany(i => choose(i));
items.SelectMany(i => choose(i));
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

This line appears to have trailing whitespace at the end. Please trim it to avoid noisy diffs and potential formatting/lint issues.

Suggested change
items.SelectMany(i => choose(i));
items.SelectMany(i => choose(i));

Copilot uses AI. Check for mistakes.

public static Option<T> ToOption<T>(this Result<T> result) => result.ToOption(logError: null);

public static Option<T> ToOption<T>(this Result<T> result, Action<string>? logError) =>
result.Match(
ok => Option.Some(ok),
Option.Some,
error =>
{
logError?.Invoke(error);
return Option<T>.None;
});

public static Result<T> ToResult<T>(this Option<T> option, Func<string> errorIfNone) =>
option.Match(s => Result.Ok(s), () => Result.Error<T>(errorIfNone()));
option.Match(Result.Ok, () => Result.Error<T>(errorIfNone()));

public static Option<string> NoneIfEmpty(this string? text)
=> text.ToOption(x => !string.IsNullOrEmpty(x));
Expand Down
17 changes: 17 additions & 0 deletions Source/Tests/FunicularSwitch.Test/OptionSpecs.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,23 @@ public void NoneIfEmpty_Text_Some()
string? target = "Hi";
var result = target.NoneIfEmpty();
result.Should().BeSome().Which.Should().Be("Hi");
}

[TestMethod]
public async Task OrElse()
{
var none = None<int>();
none.OrElse(Option<int>.None).Should().BeNone();
ShouldBeSome42(none.OrElse(42));

var some = Some(42);
ShouldBeSome42(some.OrElse(Option<int>.None));
ShouldBeSome42(some.OrElse(2));

ShouldBeSome42(await none.OrElse(() => Task.FromResult(Some(42))));
return;
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

The return; is unnecessary here and makes the local function placement confusing. Consider removing the early return and just keep the local helper at the end of the method (or move the helper above the assertions).

Suggested change
return;

Copilot uses AI. Check for mistakes.

Comment on lines +844 to +846
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

This new test only exercises the synchronous Option<T>.OrElse(Option<T>), the implicit-value case, and the Func<Task<Option<T>>> overload. It doesn't cover OrElse(Func<Option<T>>) or the Task-based overloads (including the Task<Option<T>> extension methods). Adding assertions for those overloads would prevent regressions in the newly added API surface.

Suggested change
ShouldBeSome42(await none.OrElse(() => Task.FromResult(Some(42))));
return;
// Async overload taking Func<Task<Option<T>>>
ShouldBeSome42(await none.OrElse(() => Task.FromResult(Some(42))));
// Synchronous overload taking Func<Option<T>>
ShouldBeSome42(none.OrElse(() => Some(42)));
// For Some, the factory should not be invoked; value should stay 42
ShouldBeSome42(some.OrElse(() => Some(100)));
// Task-based overloads on Option<T>
ShouldBeSome42(await none.OrElse(Task.FromResult(Some(42))));
// Task<Option<T>> extension method overloads
var noneTask = Task.FromResult(none);
ShouldBeSome42(await noneTask.OrElse(Some(42)));
ShouldBeSome42(await noneTask.OrElse(() => Some(42)));
ShouldBeSome42(await noneTask.OrElse(Task.FromResult(Some(42))));
ShouldBeSome42(await noneTask.OrElse(() => Task.FromResult(Some(42))));

Copilot uses AI. Check for mistakes.
void ShouldBeSome42(Option<int> option) => option.Should().BeSome().Which.Should().Be(42);
}

class MyClass;
Expand Down
Loading