Nested if statement with TaskEither

[adinvadim]

How is better and more clean write code with nested if statement and TaskEither?

Code block:

if userId != null:
  yes: if checkExistByUserId
      yes: createPaymentWithUser
      no: NOT_FOUND
  no: if checkExistByEmail
      yes: createPaymentWithUser
      no: createPayment

    pipe(
      TE.of(userId),
      TE.chain(() => {
        if (userId == null) {
          return pipe(
            this.checkUserExistByEmail(email),
            TE.chainW((user) => this.savePaymentWithUser(...)),
            TE.orElse((e) => {
              if (e.code === PaymentCreateErrorCode.USER_NOT_FOUND) {
                return this.savePayment(...)
              }
              return TE.left(e)
            })
          );
        } else {
          return pipe(
            this.checkUserExistByUserId(userId),
            TE.chainW((user) => this.savePaymentWithUser(...))
          )
        }
      })
    );

[steida]

Before answering, why do you have this. there? It's totally anti FP. With this, you never know if there is a hidden mutation.

As for nesting, the software is made of trees, little trees are OK, bigger trees can be refactored. Sometimes you can make pipe flat with do notation, but do you really need it? Just give that anonymous function a name a write it elsewhere. You can also replace if statement with ternary expression.

[samhh]

I've been wanting to improve my familiarity with monad transformers in Haskell and business logic like this is a great way to do that, so here's how I've modelled your problem (anyone familiar please do critique! ❤️):

type IOEither e = ExceptT e IO

newtype UserId = UserId Text
newtype Email = Email Text

data User = User

data SavePaymentWithUserErr
  = UserNotFound
  | Unknown

getUserById :: UserId -> IOEither Text (Maybe User)
getUserById = undefined

getUserByEmail :: Email -> IOEither Text (Maybe User)
getUserByEmail = undefined

savePaymentWithUser :: User -> IOEither SavePaymentWithUserErr ()
savePaymentWithUser = undefined

savePaymentWithoutUser :: IOEither Text ()
savePaymentWithoutUser = undefined

savePaymentMaybeWithUser :: User -> IOEither Text ()
savePaymentMaybeWithUser u = savePaymentWithUser u `catchE` f
  where f UserNotFound = savePaymentWithoutUser
        f Unknown      = throwE "An unknown error occurred"

savePayment :: (UserId, Email) -> IOEither Text ()
savePayment (uid, email) = getUser >>= save
  where getUser = getUserById uid <|> getUserByEmail email
        save Nothing  = savePaymentWithoutUser
        save (Just u) = savePaymentMaybeWithUser u

You don't need to know Haskell to see the difference in structure. It's much flatter, with lots of the nested parts factored out into their own small functions. This probably also ties into the other comment about the many references to this - you should favour many small functions.

As you can see there aren't any if statements. There are conditions, but they're instead modelled in terms of pattern matching, typeclass-aware constructs like <|> (from Alternative), and by making better use of the type system, specifically in opting for Maybe/Option instead of null.

In my experience, if you break your problem down into a series of function type signatures - don't even implement them yet - it will guide you towards a cleaner solution. You can see I've made use of that here where I've been able to model a domain without knowing or really caring about how getUserById or User are implemented.

If you're unfamiliar with Haskell and want to better be able to read the above, some quick tips:

• >>= is monadic chain
• data is like enums but optionally storing data, similar to how Either et al are modelled in fp-ts
• () is "unit", think of it like TypeScript's void
• Backticks make a binary function infix i.e. a function that takes two arguments can take one before and one after instead of both after

[adinvadim]

Thank you very match!

Version with using alt + Option looks great:

    pipe(
      O.fromNullable(userId),
      TE.fromOption(() => 'userId is missing'),
      TE.chainW((userId) => this.getUserByUserId(userId)),
      TE.altW(() => this.getUserByEmail(email)),
      TE.chainW((user) => this.savePaymentWithUser(...)),
      TE.orElse((e) => {
        if (e.code === PaymentCreateErrorCode.USER_NOT_FOUND) {
          return this.savePayment(...);
        }
        return TE.left(e);
      })
    );

[adinvadim]

Before answering, why do you have this. there? It's totally anti FP

My context of this script it is backend with Dependecy Injection over service contructors.

Services look like:

export class SomeService implements SomePort {
  constructor(somePersistenceService: SomePersistencePort, authService: AuthPort) {}
  
  someMethod() {
   // logic on fp-ts
  }
}

I dont write FP-only code, but all business logic write with fp-ts abstactions and pure functions

[steida]

https://dev.to/gcanti/getting-started-with-fp-ts-reader-1ie5