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.

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

Powers of two #8

Merged
merged 5 commits into from
Feb 15, 2021
Merged

Powers of two #8

merged 5 commits into from
Feb 15, 2021

Conversation

MihailKulikov
Copy link
Owner

No description provided.

Copy link

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

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

В целом всё так, есть только рекомендации на будущее.

Comment on lines +5 to +6
if n < 0 || m < 0 then None else
let two = BigInteger.One + BigInteger.One

Choose a reason for hiding this comment

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

Я даже немного удивлён, что это компилится, хотя если так подумать, в этом есть смысл. Всё же мне больше нравится наличие отступа после else (а лучше и else на отдельной строчке).

open System

let powersOfTwo n m =
if n < 0 || m < 0 then None else

Choose a reason for hiding this comment

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

Я бы сказал, что тут лучше исключение бросать. Вызывающий сам вполне может проверить параметры, и если он налажал, пусть исключение ловит. Иначе неудобно каждый раз после вызова на Some/None проверять. Хотя исключения не очень в функциональном стиле, так что идеологически Ваше решение правильнее

@MihailKulikov MihailKulikov merged commit 0566423 into main Feb 15, 2021
@MihailKulikov MihailKulikov deleted the hw1.4 branch February 15, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants