Skip to content
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

Jump to bottom

Use exists instead of filter + match on [] #1212

Merged
merged 1 commit into from May 21, 2016
Merged

Use exists instead of filter + match on [] #1212

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/fsharp/TypeChecker.fs
View file
Open in desktop
Expand Up @@ -2112,9 +2112,9 @@ module GeneralizationHelpers =
match tp.Constraints |> List.partition (function (TyparConstraint.CoercesTo _) -> true | _ -> false) with
| [TyparConstraint.CoercesTo(cxty,_)], others ->
// Throw away null constraints if they are implied
match others |> List.filter (function (TyparConstraint.SupportsNull(_)) -> not (TypeSatisfiesNullConstraint cenv.g m cxty) | _ -> true) with
| [] -> Some cxty
| _ -> None
if others |> List.exists (function (TyparConstraint.SupportsNull(_)) -> not (TypeSatisfiesNullConstraint cenv.g m cxty) | _ -> true)
Copy link
Contributor

Choose a reason for hiding this comment

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

would we prefer this style:

// Throw away null constraints if they are implied 
let hasTypesWithNullConstraint =
    others 
    |> List.exists (
        function
        | (TyparConstraint.SupportsNull(_)) -> not (TypeSatisfiesNullConstraint cenv.g m cxty) 
        | _ -> true
    ) 
if hasTypesWithNullConstraint 
then None
else Some cxty

I know most of the compiler has long lines, but from my perspective it really ease the flow of reading when we adopt shorter lines / expressions, introducing / naming a variable also helps

Copy link
Contributor

Choose a reason for hiding this comment

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

@smoothdeveloper Per CONTRIBUTING,md, that sort of thing would be a separate cleanup PR (though it's fair to say that reducing line length isn't really a cleanup aim at the moment)

then None
else Some cxty
| _ -> None


Expand Down