-
Notifications
You must be signed in to change notification settings - Fork 140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Java: The Sodium API is too dangerous. #72
Comments
It should only ever be used when you absolutely have to use it. I don't agree that it is more dangerous than it should be. Maybe we need to talk more about what you are trying to do. |
It sounds like all but the third issue are the result of crossing the I may have to agree that 'defer' is a bit of a strange beast; it seems to On Tue, Aug 4, 2015 at 8:32 AM, the-real-blackh notifications@github.com
|
Exactly. The interface between FRP and non-FRP does require some care, and so the "danger" of these things is there to help you. The best example of this is send() inside listen(). I understand your reservations about defer(), though strictly speaking it is denotational (except that the implementation doesn't match the semantics until I have fixed issue #71). It was intended for certain operational cases, not for general use. I think the solution is to put it into Operational along with updates() and value(). |
It seems I found a solution: to make special version of public fun sendInExplicitTx(a: A) {
val tx = Transaction()
try {
send(tx, Value(a))
} finally {
tx.close()
}
} It solves the problems 2, 3 and 4. |
This is a bad, bad idea, for two reasons:
I urge you not to go down this path. |
Yes it allows user to write its own primitive-like operations, but them will be not the real primitives. The It easier to understand for user. The order of computations will be defined by this rule.
|
If send() is the start of a computation, then you won't fall foul of "2. send inside listen - Exception". You also shouldn't use it inside a map() function. |
You right, I think about it. |
I understood one thing: the problem with Other idea: how about throw Or just provide user the ability to create explicit transaction? |
I suggest you do it this way: .listen((x) -> { new Thread(() -> { ... do I/O ... .send(y); }).start(); } |
Yes I can, but as I described in #64, I meet with unpredictable exception with codel like this. I can fix it here, but I can not guarantee that not meet this problem again. I already spent in this place about 10 hours. The code is simple, but not easy. Actially I want to |
Ouch... not like this. I want to |
It can be done without an unpredictable exception. I'll try to help. Are you sometimes not launching a new thread? You must guarantee to do so. |
Yes, man can walk through mine field without explosion, but it is wrong way. |
Sodium should be very free from anything non-deterministic. What exception are you getting? |
Here is the solution of my problem in Android:
Is it legal? This code is not multithread. And it solution is for Android only. Actually this code eqivalent to my suggestion to make |
What does handler.post do? |
It just queue of actions like main application queue. val handler = Handler()
val stream = ...
val sink = ...
stream.listen {
val result = it.value
handler.post {
sink.send(result)
}
}
handler.post {
stream.send(Unit)
} |
|
Thanks - Not very familiar with that java.util.concurrent stuff. So this code of yours would definitely be legal Sodium, and it shouldn't throw an exception. val handler = Handler()
stream.listen {
val result = it.value
handler.post {
sink.send(result)
}
} |
Just small experiment: SodiumFRP/sodium-kotlin@1a0f8fd#diff-9c15ed34aea5dff2bbe7206855238731L120 The Also notice the |
Here some common errors I meet in my code:
addClanup
for the result oflisten
- the computations graph will be GCed.send
insidelisten
- Exceptiondefer
in theflatMap
example - the event will be lostsend
second time in transaction - ExceptionThe
send
is most dangerous method in lib. Though must be the safest. We need to do something with it.The text was updated successfully, but these errors were encountered: