tofu-tf / tofu

Functional programming toolbox
https://tofu-tf.github.io/tofu/
Apache License 2.0
535 stars 92 forks source link

Suggestion for changing `Logging` hierarchy #1268

Open geny200 opened 4 months ago

geny200 commented 4 months ago

I looked at the hierarchy of Logging and ServiceLogging, and I don't realize why Logging extends from ServiceLogging, not the other way round. I suggest swapping them, also removing the method def to[Svc2]: ServiceLogging[F, Svc2] (because this method does not realy change the name of the logger).

suggestion:

trait Logging[F[_]] extends LoggingBase[F] {
  final def widen[G[a] >: F[a]]: Logging[G] = this.asInstanceOf[Logging[G]]

  def asLogging: Logging[F] = this
}

trait ServiceLogging[F[_], Service] extends Logging[F] {
  @deprecated
  final def to[Svc2]: ServiceLogging[F, Svc2] = this.asInstanceOf[ServiceLogging[F, Svc2]]
}

also change logs factory:

trait Logs[+I[_], F[_]] extends LogsVOps[I, F] {
  // Here is the difference
  def forService[Svc](implicit Svc: ClassTag[Svc]): I[ServiceLogging[F, Svc]] = 
    byName(Svc.runtimeClass.getName()).asInstanceOf[I[ServiceLogging[F, Svc]]]

  // Here is the difference
  def byName(name: String): I[ServiceLogging[F, Nothing]]

  final def biwiden[I1[a] >: I[a], F1[a] >: F[a]]: Logs[I1, F1] = this.asInstanceOf[Logs[I1, F1]]

  final def service[Svc: ClassTag]: I[ServiceLogging[F, Svc]] = forService[Svc]

  final def of[Svc[_[_]]](implicit tag: ClassTag[Svc[HKAny]]): I[ServiceLogging[F, Svc[HKAny]]] =
    service[Svc[HKAny]]
}

The reason why I propose this change is because now the base class for logging is ServiceLogging[F, Svc] (base LoggingBase is deprecated), so to add correct syntax impl for logging, you need to require implicit ServiceLogging[F, Any], not Logging[F] which is not obvious.

dos65 commented 3 months ago

@geny200 Thanks for proposal. However I would avoid making such changes as it will add compat issues.

The reason why I propose this change is because now the base class for logging is ServiceLogging[F, Svc] (base LoggingBase is deprecated), so to add correct syntax impl for logging, you need to require implicit ServiceLogging[F, Any], not Logging[F] which is not obvious.

I think that introducing incompatible changes will bong more problems that and issue you describe in this motivation. wdyt?

geny200 commented 3 months ago

I think that introducing incompatible changes will bong more problems

The only incompatible change here will be that the Logging type will no longer have the "to" method (I consider that this method should not exist in its current form, because it provides a cheat for types), so we can add "to" method for binary compatibility to Logging and immediately set as deprecated