Skip to content
This repository has been archived by the owner on May 29, 2023. It is now read-only.

Commit

Permalink
Fix Just::concat()
Browse files Browse the repository at this point in the history 
As spotted by the keen-eyed @shadowhand in #4, this definition was
broken. Now amended, phew :)
  • Loading branch information
Tom committed May 30, 2017
1 parent 535bd04 commit 9d57a59
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/Constructor/Just.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public function concat(Maybe $that) : Maybe
return $this;
}

return new Maybe(
return Maybe::of(
$this->value->concat(
$that->fork(null)
)
Expand Down

3 comments on commit 9d57a59

@shadowhand
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this actually fixes the problem... how could $this->value be an object (looks like it is meant to be a Maybe)?

@i-am-tom
Copy link

Choose a reason for hiding this comment

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

$this is the Just value, but $this->value is the value within the Just. Given that the other, at this point, is a Just, we could probably write $this->value->concat($that->value), thinking about it, but the type-checker's not clever enough to spot that they're both Just, so the $value would need to become a protected prop of Maybe, which seems counter-intuitive...

Anyway, tl;dr, Maybe is a Semigroup if the contained value is a Semigroup (i.e. has a valid concat function), so the inner value needs to be an object!

@shadowhand
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, I missed that this would only apply to a maybe-wrapped-maybe.

Please sign in to comment.