Utility improvements #85
Conversation
* Add connect & transact methods directly on Transactor * Add commonly used classes under magnum.common.* export, so users don't get swamped by every class living in the magnum package * Add magzio.Transactor.semaphore to save memory when not using the Virtual-Thread based blocking executor * Add .uninteruptable to magzio.Transactor.connect method like it is for Transactor, remove null check
| import scala.reflect.ClassTag | ||
| import scala.util.control.{ControlThrowable, NonFatal} | ||
|
|
||
| class Transactor private ( |
There was a problem hiding this comment.
Allows for more JIT optimisation and it's not supposed to be extended anyway
| class Transactor private ( | |
| final class Transactor private ( |
There was a problem hiding this comment.
There's actually not a single final keyword in the magnum codebase despite almost every class NOT being designed for extension. So to be consistent, we would want to make this change almost everywhere. (I'd like to defer to separate MR for that reason).
The two exceptions are Repo and ImmutableRepo, which are declared open. (I do prefer composition over inheritance when using these, and will update the docs to show an example , but it's nice to support extension like you can do in Spring Data).
https://docs.scala-lang.org/scala3/reference/other-new-features/open-classes.html
My thought is that open is much better than final, since a whitelist is more restrictive than blacklist, and less work. And in Scala 3.4+ extending a class that isn't open results in a feature warning.
I don't think I agree that adding the final keyword to a class will improve performance (at least, I can't seem to find any articles about it online). I do agree with final fields (haha, but even then the JVM can have trust issues.. https://shipilev.net/jvm/anatomy-quarks/17-trust-nonstatic-final-fields/ )
| */ | ||
| case class Transactor( | ||
| /** Datasource to be used */ | ||
| class Transactor private ( |
There was a problem hiding this comment.
| class Transactor private ( | |
| final class Transactor private ( |
magnum-zio/src/main/scala/com/augustnagro/magnum/magzio/Transactor.scala
Outdated
Show resolved
Hide resolved
magnum-zio/src/main/scala/com/augustnagro/magnum/magzio/Transactor.scala
Outdated
Show resolved
Hide resolved
magnum-zio/src/main/scala/com/augustnagro/magnum/magzio/Transactor.scala
Show resolved
Hide resolved
magnum-zio/src/main/scala/com/augustnagro/magnum/magzio/Transactor.scala
Outdated
Show resolved
Hide resolved
magnum-zio/src/main/scala/com/augustnagro/magnum/magzio/Transactor.scala
Outdated
Show resolved
Hide resolved
|
Thanks for the review @guizmaii . I'll go ahead and merge; we can address the |
Closes #76
Closes #77
Closes #74
Todo: Update readme MR
please review @guizmaii