Skip to content

Refactor code using onLeft and onNothing #4815

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

Merged
merged 2 commits into from
Feb 8, 2023

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Jan 20, 2023

Using onLeft and onNothing gives more opportunities for straight line code. Examples of this are marked.

Modifying the code elsewhere as well for general consistency.

This PR is divided into multiple commits to group related changes for easy review.

Note, throwE from Control.Monad.Trans.Except is the same as left from Control.Monad.Trans.Except.Extra.

@newhoggy newhoggy marked this pull request as ready for review January 20, 2023 08:25
@newhoggy newhoggy force-pushed the newhoggy/use-onLeft-in-more-places branch 6 times, most recently from 3195ef7 to 5a6fe9e Compare January 20, 2023 10:16
@newhoggy newhoggy changed the title Using onLeft combinator in more places Refactor code using onLeft and onNothing Jan 20, 2023
@newhoggy newhoggy force-pushed the newhoggy/use-onLeft-in-more-places branch from 5a6fe9e to 687701a Compare January 20, 2023 10:22

writeFilteredUTxOs sbe mOutFile result
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Straight line code.

-- so our ondisk op cert counter must be greater than or
-- equal to what is in the node state
Just ptclStateCounter -> return (OpCertOnDiskCounter onDiskOpCertCount, Just $ OpCertNodeStateCounter ptclStateCounter)
Nothing -> return (OpCertOnDiskCounter onDiskOpCertCount, Nothing)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Straight line code.


let qInMode = QueryInEra eInMode . QueryInShelleyBasedEra sbe $ QueryDebugLedgerState

result <- executeQuery era cModeParams localNodeConnInfo qInMode
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Straight line code.

Nothing -> left $ ShelleyQueryCmdEraConsensusModeMismatch (AnyConsensusMode cMode) anyE
case cMode of
CardanoMode -> eligibleWriteProtocolStateConstaints sbe $ writeProtocolState mOutFile result
mode -> left . ShelleyQueryCmdUnsupportedMode $ AnyConsensusMode mode
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Straight line code.


result <- executeQuery era cModeParams localNodeConnInfo query

writeStakeAddressInfo mOutFile $ DelegationsAndRewards result
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Straight line code.

)
)

liftIO . LBS.putStrLn $ encodePretty poolStates
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Straight line code.


let query = QueryInEra eInMode . QueryInShelleyBasedEra sbe $ QueryStakeDistribution

result <- executeQuery era cModeParams localNodeConnInfo query
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Straight line code.

@newhoggy newhoggy force-pushed the newhoggy/use-onLeft-in-more-places branch from 687701a to d47a3e3 Compare January 24, 2023 17:25
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Nice work 👍 . Just replace throwE with left and I'll approve. You can squash all the commits if you like since its just a refactoring and not difficult to understand what is going on.

@@ -1,12 +1,10 @@
{-# LANGUAGE OverloadedStrings #-}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@newhoggy newhoggy requested review from Jimbo4350 and removed request for LudvikGalois January 24, 2023 21:11
@newhoggy
Copy link
Contributor Author

All occurrences of throwE replaced with left.

@newhoggy newhoggy force-pushed the newhoggy/use-onLeft-in-more-places branch 3 times, most recently from 58e4736 to 1af4803 Compare January 28, 2023 18:32
@newhoggy newhoggy force-pushed the newhoggy/use-onLeft-in-more-places branch from 1af4803 to 6d731c4 Compare February 3, 2023 17:34
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

LGTM!

@newhoggy newhoggy force-pushed the newhoggy/use-onLeft-in-more-places branch from 6d731c4 to f0571e9 Compare February 8, 2023 06:36
@newhoggy
Copy link
Contributor Author

newhoggy commented Feb 8, 2023

bors r+

iohk-bors bot added a commit that referenced this pull request Feb 8, 2023
4815: Refactor code using onLeft and onNothing r=newhoggy a=newhoggy

Using `onLeft` and `onNothing` gives more opportunities for straight line code.  Examples of this are marked.

Modifying the code elsewhere as well for general consistency.

This PR is divided into multiple commits to group related changes for easy review.

Note, `throwE` from `Control.Monad.Trans.Except` is the same as `left` from `Control.Monad.Trans.Except.Extra`.

Co-authored-by: John Ky <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 8, 2023

Timed out.

@Jimbo4350
Copy link
Contributor

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 8, 2023

Build succeeded:

@iohk-bors iohk-bors bot merged commit c8862fe into master Feb 8, 2023
@iohk-bors iohk-bors bot deleted the newhoggy/use-onLeft-in-more-places branch February 8, 2023 11:29
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