Feedback sought on Stellar SDK for Scala
15 views
Skip to first unread message
Jem
Mar 7, 2018, 1:39:00 AM3/7/18
to scala...@googlegroups.com
Hi all.
I'm about to notify the Stellar dev community that the SDK for Scala is ready to use. I'm looking for feedback from the Scala user group on the SDK project.
Stellar is a distributed ledger network designed to make cross-border payments as ubiquitous as email and also aims to assist the world's unbanked. I'm drawn to the network for these and other reasons (which I'm happy to elaborate on if anyone is interested).
There exist several SDKs already, including an official Java one. In the project readme I've make a bullet-point argument as to why a Scala developer might prefer the Scala SDK.
As Scala aficionados and experts, I am interested in gathering your opinions on the project before I announce it. Anything is fair game. Is the documentation clear, or terrible? Is anything you'd expect to be present missing? Would scaladoc help? Are the examples clear and succinct? etc.
If you can spare any time to review it, please feel free to give your feedback via email (this thread or privately) or in slack.
Thank you!
Jem
Tom Adams
Mar 7, 2018, 2:46:50 AM3/7/18
to Scala Melb
I had a quick look, and it looks really good! But, I know nothing about Stellar, so grain of salt, etc. Tests look clear, code looks clean. I personally prefer to pass Gen instances rather than Arbitrary's, but the test code looks super clear & it's consistent.
I did see some formatting differences with { in https://github.com/Synesso/scala-stellar-sdk/blob/master/examples/03-operation_history_for_account.scala :)
Is there anything specific you think it's missing? I guess the only thing I'd not be too sure about is how it works :) But you'd need to use it in anger I'd guess to get a feel for how easy it is to code against.
Tom
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
tom adams
e:tomjadams<at>gmail.com
e:tomjadams<at>gmail.com
Ben Hutchison
Mar 7, 2018, 3:06:25 AM3/7/18
to scala...@googlegroups.com
+1
How Stellar itself works is the main uncertainty for me.
Ken Scambler
Mar 7, 2018, 3:27:19 AM3/7/18
to scala...@googlegroups.com
I have comments! I'm on the run now though, so it'll have to wait a bit :)
Jem
Mar 7, 2018, 3:27:57 AM3/7/18
to scala...@googlegroups.com
Thank you both for your feedback. I've now formatted the project. I have to dig up where Intellij keeps the Scala formatting rules because I don't like the redundant braces it puts around
yield {
someResult
}
At the most general level, Stellar exposes an API where you can submit transactions and query the network state. This library provides a Scala wrapper to these endpoints. Operations batched in transactions vary, but are primarily focused around paying or making offers to trade various assets.
As to what I might be missing, it's in the unknown/unknown quadrant. But I know I probably need to improve documentation. I wonder if more examples is the right approach, or scaladoc, or a sample app ...
Ken Scambler
Mar 7, 2018, 8:07:22 AM3/7/18
to Unname
Hey Jem,
I don't know anything about the domain, so I'll just look at some code things.
There's a bunch of common traps that "Scala astroturf on a stable Java lib" libs fall into, so this is where I'm going to start. Often, the underlying Java framework will be blazingly fast and really really stable, because a bazillion people have been using and maintaining it for ten years. The risk is that our Scala-toting lone wolf will splatter cosmetic syntax vanities around that mean:
- The result is more complex than the original
- There is more to learn, because the user must also learn the underlying library
- It loses the speed of the Java lib, because no thought has been put to performance
- It will fall out of date, because it is maintained with far less energy and infrequently updated
At a glance, it looks like you've made sensible data modelling decisions, and exercised a lot of restraint with regard to the flamboyant implicitry and DSL silliness that so many fall prey to. That's good. It occurs to me that a millisecond is an eternity on a payment platform, so you may wish to profile or benchmark your solution, to understand what effect on performance you are having.
There are some things that can be improved:
1.
You are using traits ByteArrays and XDRPrimitives as bags of functions, which classes extend when they want to use them. This is a bad pattern, because everything that extends these exports these methods all over again. You don't want the public API of your case classes and their companion objects polluted with hundreds of utility methods. It's worse than that, because for all a user knows, a subtype could be overriding the methods, so that Asset.hexString and myAsset.hexString do different things. Scalaz & Cats do something similar, but they do it to re-export the functions to make importing easier for users, not as a weird complicated alternative to imports. I would make ByteArrays and XDRPrimitives objects and either import them, or better still use them directly: ByteArrays.hexString etc.
2.
It's not a bad thing to have assertions in your constructors to guarantee valid state, but it's better to use the type system if you can. For instance, if there is something like:
case class MyClass(code: String, ...) {
assert(code.length == 5 && code.startsWith("P"))
}
This is really saying that 'code' is not a string at all; it cannot contain arbitrary text, it is not interchangeable with other types of code, and there are constraints on what it can be. If you push this logic into a Code type, then merely by mentioning 'Code', MyClass is completely typesafe and will be correct by construction and not throw errors. 'Code' might look like:
case class Code(value: String) {
assert(code.length == 5 && code.startsWith("P"))
}
or even better, use a "smart constructor":
case class Code private (value: String)
object Code {
def apply(value: String): Option[Code] = ...
}
3.
Use def rather than val in traits. Vals are unnecessarily specific and can cause strange initialisation-order shenanigans if you layer traits as mixins.
That's all I've got for now. Anyway, good on you for doing this - I hope your library is a success!
Cheers,
Ken
I don't know anything about the domain, so I'll just look at some code things.
There's a bunch of common traps that "Scala astroturf on a stable Java lib" libs fall into, so this is where I'm going to start. Often, the underlying Java framework will be blazingly fast and really really stable, because a bazillion people have been using and maintaining it for ten years. The risk is that our Scala-toting lone wolf will splatter cosmetic syntax vanities around that mean:
- The result is more complex than the original
- There is more to learn, because the user must also learn the underlying library
- It loses the speed of the Java lib, because no thought has been put to performance
- It will fall out of date, because it is maintained with far less energy and infrequently updated
At a glance, it looks like you've made sensible data modelling decisions, and exercised a lot of restraint with regard to the flamboyant implicitry and DSL silliness that so many fall prey to. That's good. It occurs to me that a millisecond is an eternity on a payment platform, so you may wish to profile or benchmark your solution, to understand what effect on performance you are having.
There are some things that can be improved:
1.
You are using traits ByteArrays and XDRPrimitives as bags of functions, which classes extend when they want to use them. This is a bad pattern, because everything that extends these exports these methods all over again. You don't want the public API of your case classes and their companion objects polluted with hundreds of utility methods. It's worse than that, because for all a user knows, a subtype could be overriding the methods, so that Asset.hexString and myAsset.hexString do different things. Scalaz & Cats do something similar, but they do it to re-export the functions to make importing easier for users, not as a weird complicated alternative to imports. I would make ByteArrays and XDRPrimitives objects and either import them, or better still use them directly: ByteArrays.hexString etc.
2.
It's not a bad thing to have assertions in your constructors to guarantee valid state, but it's better to use the type system if you can. For instance, if there is something like:
case class MyClass(code: String, ...) {
assert(code.length == 5 && code.startsWith("P"))
}
This is really saying that 'code' is not a string at all; it cannot contain arbitrary text, it is not interchangeable with other types of code, and there are constraints on what it can be. If you push this logic into a Code type, then merely by mentioning 'Code', MyClass is completely typesafe and will be correct by construction and not throw errors. 'Code' might look like:
case class Code(value: String) {
assert(code.length == 5 && code.startsWith("P"))
}
or even better, use a "smart constructor":
case class Code private (value: String)
object Code {
def apply(value: String): Option[Code] = ...
}
3.
Use def rather than val in traits. Vals are unnecessarily specific and can cause strange initialisation-order shenanigans if you layer traits as mixins.
That's all I've got for now. Anyway, good on you for doing this - I hope your library is a success!
Cheers,
Ken
Jem
Mar 7, 2018, 6:17:12 PM3/7/18
to scala...@googlegroups.com
Thanks Ken. This is very helpful. I agree with all your points.
Re Smart Constructors - it's a pattern I use and like, but when it forms a frequently used part of the API I'm concerned about ease-of use. e.g. e.g. Assets. The domain is a bit weird about them. 1-4 characters is one type and 5-12 characters is another type and anything else is invalid. These objects would be very commonly constructed. I can have a smart constructor and force users to juggle Trys, or I can assume they know what they're doing and let any mistakes crash. I will move to smart constructors and see how it feels.
As an aside, I was tempted to astroturf initially but decided on a re-write. The java sources in the project are XDR auto-generated classes - there does not yet exist a Scala generator for XDR.
Cheers
Jem
Ken Scambler
Mar 7, 2018, 7:08:00 PM3/7/18
to scala...@googlegroups.com
Agree with your point about smart constructors. YMMV, and up to you where the boundary between convenience and principle is of course.
Tom Adams
Mar 7, 2018, 7:47:51 PM3/7/18
to Scala Melb
Interestingly, we've been going through the latest version of Effective Java at work, and he makes recommendations for effectively the same thing re. constructors :) Of course the names are longer as they're more meaningful ;)
Ben Hutchison
Mar 7, 2018, 9:04:53 PM3/7/18
to scala...@googlegroups.com
Im in the smart constructors camp myself. Keeps errors closer to their source.
I think it ends up about the same amount of total difficulty, providing the programmer is comfortable using for-notation. Instead of try-catch, you put everything into a Try-based for-expression and handle the Valid and Invalid cases at the end.
Error handling is the gateway drug for monadic effects.
-Ben
Chris Myers
Mar 7, 2018, 9:13:24 PM3/7/18
to scala...@googlegroups.com
As an aside, having private constructors for case classes in Scala is a little flaky and can be subverted. We often use the sealed abstract case class pattern as documented here: https://gist.github.com/tpolecat/a5cb0dc9adeacc93f846835ed21c92d2
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
Jem
Mar 8, 2018, 1:15:24 AM3/8/18
to scala...@googlegroups.com
Reading the comments on that and I'm a bit confused. It says there's a problem with `case class C private` and how to work around it with `sealed abstract case class`. But later in the comments SethTisue seems to be saying you no longer need the workaround because now we have `case class C private` ...
SethTisue commented on 4 Jun 2017
As of Scala 2.11.11 (with |
What am I missing?
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
Chen Harry
Mar 8, 2018, 2:58:01 AM3/8/18
to scala...@googlegroups.com
Jem, excellent work. I am just curious why you choose Try instead of IO to handle side-effect? given that Try breaks monad law.
Cheers,
Harry
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
Jem
Mar 8, 2018, 3:42:26 AM3/8/18
to scala...@googlegroups.com
Thanks Harry.
I wanted to stick to the standard lib to make it easier to adopt. (But I don't know my audience, if I have one... perhaps more would prefer 3rd party FP libraries than would be turned away from it...?)
Which side-effecting method are you talking about? The only side-effects I can think of are HTTP related, and these are all Futures.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+unsubscribe@googlegroups.com.
Chris Myers
Mar 8, 2018, 5:03:24 PM3/8/18
to scala...@googlegroups.com
It certainly looks better than used to be! Though as I understand it, case classes still have a copy method which can undermine the constructor/apply method. I tried this in the REPL:
scala> :paste
// Entering paste mode (ctrl-D to finish)
case class Hi private (message:String)
object Hi { def apply(msg:String):Option[Hi] = Some(new Hi(msg)) }
// Exiting paste mode, now interpreting.
defined class Hi
defined object Hi
scala> Hi("Chris")
res5: Option[Hi] = Some(Hi(Chris))
scala> res5.get.copy(message="Jem")
res6: Hi = Hi(Jem)
There is a suggestion that you can zero out the copy method, though I haven't tried that yet.
Cheers,
Chris
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
To post to this group, send email to scala...@googlegroups.com.
Visit this group at https://groups.google.com/group/scala-melb.
For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "Melbourne Scala User Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scala-melb+...@googlegroups.com.
Chris Myers
Mar 8, 2018, 5:07:19 PM3/8/18
to scala...@googlegroups.com
Yeah it looks like zeroing out the copy method works too:
scala> :paste
// Entering paste mode (ctrl-D to finish)
case class Hi private (message:String) { private def copy() = () }
object Hi { def apply(msg:String):Option[Hi] = Some(new Hi(msg)) }
// Exiting paste mode, now interpreting.
defined class Hi
defined object Hi
scala> Hi("Chris")
res8: Option[Hi] = Some(Hi(Chris))
scala> res8.get.copy(message = "Ken")
<console>:16: error: method copy in class Hi cannot be accessed in Hi
res8.get.copy(message = "Ken")
^
<console>:16: error: not found: value message
res8.get.copy(message = "Ken")
^
Chen Harry
Mar 8, 2018, 5:24:14 PM3/8/18
to scala...@googlegroups.com
I tried this:
sealed abstract case class Hi private (msg: String)
object Hi {
def apply(s: String): Option[Hi] = Some(new Hi(s) {})
}
val hi = Hi("msg")
and the copy method is gone from hi... I thought it is tpolecat who first proposed the 'sealed abstract case class' thing.. :)
Chris Myers
Mar 8, 2018, 5:29:03 PM3/8/18
to scala...@googlegroups.com
Yep,
But you don't need the `private` keyword if you use `sealed abstract` because only things in that source file can extend it.
I think for now I'll stick with the sealed abstract approach, but YMMV.
Chen Harry
Mar 8, 2018, 5:36:58 PM3/8/18
to scala...@googlegroups.com
you're right. thanks Chris.
Reply all
Reply to author
Forward
0 new messages