Weaken Either monoid dependencies to only require a semigroup on left

[mjvmroz]

Either is a right-biased sum, so no empty is needed on its left side for it to form a Monoid: just a combine. I've weakened the dependency to reflect this. Since Monoid<A> : Semigroup<A>, this is a non-breaking change.

[mjvmroz]

Oop sorry, I'm not used to Github's model. That force push was just to try to force a build on my end after enabling actions for my fork. I see now that the build has to run here, and that that has to be approved; makes sense.

[raulraja]

Thanks, @mjvmroz.

looks great and can probably make it to the next release but be aware we are planning a deprecation of Monoid and other Typeclasses that currently provide no added value.

In this case, for example, Semigroup can be just expressed as (A, A) -> A. All methods where we need a semigroup or monoid can be implemented without them by passing just the function or neutral element A where needed.

[nomisRev]

Thank you @mjvmroz, and congrats on your first-contribution! 🥳

[mjvmroz]

@raulraja Oh, interesting – I'm making fairly extensive use of both in a project at the moment. WhileSemigroup<A> can certainly be reduced to (A, A) -> A and Monoid<A> to (A, Semigroup<A>), the value of both is in the laws that the names imply, and the associated Kotest-integrated law tests in arrow-core-test. In my case I'm building a commerce system and need to guarantee that bundles of numbers roll up correctly in various structures, so a lot of my stuff has Monoid instances. I'm sure there are similar applications in other correctness-critical spaces where Kotlin/Arrow are commonplace too, like the enterprise SaaS identity domain.

I've followed the progression of Arrow for quite a while now. I thought the decision to move away from emulated HKTs was a good one, and I 100% get it if these typeclass encodings and tests don't really feel too relevant to the project anymore, but I think they're useful enough to earn themselves and their tests a home somewhere.

I'm certainly not going to argue that that's a good enough reason for any of this to continue living here; just looking to understand the intention and a rough timeline for this change. Are you planning to nuke this all entirely, or just move it out of arrow-core? If necessary, I'll happily just put together a small package of my own.

[raulraja]

@mjvmroz we have not decided the actual outcome of the refactoring over those yet but generally speaking, most places where they were in use in Arrow are gone because of changes to the methods that depended on them.
Do you have some example code of your usage where we can judge if it would be better expressed differently with Arrow 2.0, thanks!

[mjvmroz]

Sure, here's one example from a sales/quoting system. Quotes contain parameterised "offering" instances, with each offering comprising a collection of parameterised items/products. Each item needs to have a billing schedule calculated, have a total price rolled up from that, and register the currency value of each applied discount. In this specific case, all of that forms a PriceSummary. Those PriceSummarys are then rolled up at an offering level, and then again at a quote level, all via a monoid instance:

data class PriceSummary(
    // `Price` itself is a composite structure with its own monoid instance
    val price: Price,
    val billingSchedule: Map<LocalDate, Price>,
    val discountValues: Map<DiscountId, BigDecimal>,
) {
    companion object : Monoid<PriceSummary> {
        override fun empty() = PriceSummary(Price.empty(), emptyMap(), emptyMap())

        private val scheduleMonoid = Monoid.map<LocalDate, Price>(Price)
        private val discountValuesMonoid = Monoid.map<DiscountId, BigDecimal>(Monoid.bigDecimalSum)
        override fun PriceSummary.combine(b: PriceSummary) = PriceSummary(
            price = price.combine(b.price),
            billingSchedule = scheduleMonoid.run { billingSchedule.combine(b.billingSchedule) },
            discountValues = discountValuesMonoid.run { discountValues.combine(b.discountValues) },
        )
    }
}

abstract class QuoteCalculator {
    // `PricingContext` holds onto a bunch of data that currently has to be pulled
    // from quotes by foreign reference, allowing us to isolate effects for testing.
    protected abstract fun buildPricingContext(quote: Quote): PricingContext
    private fun PricingContext.itemPrice(item: QuoteItem, period: DateSpan): PriceSummary = /* ... */

    // `QuoteBreakdown` mirrors `Quote`'s structure, but contains
    // arbitrary data associated with offering/item identifiers.
    fun priceQuote(quote: Quote): QuoteBreakdown<PriceSummary> = buildPricingContext(quote).run {
        // `foldOnItems` second parameter is demanding a monoid instance for `QuoteBreakdown<*>`'s type param
        QuoteBreakdown.foldOnItems(quote, PriceSummary) { item, offering -> itemPrice(item, offering.dateSpan) }
    }
}

We have similar needs when running other calculations with respect to quotes also using QuoteBreakdown (e.g. projected revenue, which is not purely a function of price for a given contract), as well as price-related structures in other flows like billing. Explicitly defining and demanding Monoids for our business logic structures is highly appealing because it sends a very clear message about the laws the functions should obey, and makes it super easy to make sure that they do with free law tests.

I didn't realise that Arrow's remaining typeclass stuff was more or less a relic and on the way out, but given the project's progression, it's not too surprising. And I do suspect that an extreme minority of Arrow consumers are using this stuff. So I will stress that while I value a formal encoding of Monoid in my system, I don't necessarily think that Arrow should own it. In any case, it's not much code and has a negligible maintenance cost, so I'm entirely happy to define it myself.

[raulraja]

thanks @mjvmroz for the example and sorry for the late reply, I've been swamped with work in the last few weeks and I just saw this.

I tried encoding your example to see what it would look like without type classes.

public data class LocalDate(val value: Long)
public data class DiscountId(val value: Long)
public object PricingContext

public data class Price(val value: Double) {
  /**
   * Semigroup combine
   */
  public operator fun plus(other: Price): Price =
    Price(value + other.value)

  public companion object {
    /**
     * Monoid empty
     */
    public val empty: Price = Price(0.0)
  }
}

public data class PriceSummary(
  val price: Price,
  val billingSchedule: Map<LocalDate, Price>,
  val discountValues: Map<DiscountId, Double>,
) {

  public operator fun plus(other: PriceSummary): PriceSummary =
    PriceSummary(
      price = price + other.price,
      billingSchedule = billingSchedule + other.billingSchedule,
      discountValues = discountValues + other.discountValues
    )

  public companion object {
    public val empty: PriceSummary = PriceSummary(Price.empty, emptyMap(), emptyMap())
  }
}

I think keeping Monoid as a structure or even having a formal definition of it, whether in Arrow or your project, is helpful whenever you use polymorphism or have generic functions that refer to Typeclass<A> where A is a type argument.

In your example, since all the types are concrete and the code referring to them is also concrete, it's more accessible to add ad-hoc operators over the types, which are easier to call and deal with than passing type class instance value arguments that satisfy type class injection requirements. If Kotlin had implicits and type class injection, it would be different since you could magically use empty and combine where needed if the types had instances derived or in scope for those types.

We are inclined to remove these type classes because, in most use cases, they are applied over concrete models and not used in polymorphism, resulting in more complicated code. This is different in Scala cats and others where there is considerable generic infra relying on these and taking advantage that they magically get injected by implicits and auto derived in some cases by the scala compiler, I think there it makes sense. I think in Kotlin, type classes get in the way due to a lack of implicit support, and the language is going in a different direction with context receivers.