# Casper SDK Scala feedback part 1

Link: https://github.com/caspercommunityio/casper-scala-sdk.
Commit hash used: `070423e1afaf58fb347b873831de63c076b97106`

## Error handling

#### Throwing exceptions.

##### files:
* `RPCCommand.scala`
* `RPCService.scala`
* `HttpRPCService.scala`
* `Crypto.scala`
* `AccountHashDeserializer.scala`
* `CLPublicKeyDeserializer.scala`
* `CLTypeDeserialiser.scala`
* `CLValueDeSerializer.scala`
* `DeployExecutableDeserializer.scala`
* `HashDeserializer.scala`
* `RPCResultDeserializer.scala`
* `SignatureDeserializer.scala`
* `URefDeserializer.scala`
* `JsonConverter.scala`

Throwing an exception is a side-effect and this violates the functional programming paradigm.

Scala does not have a concept of *checked exception*, unlike Java, so using them in Scala code has not only much worse UX but it's more dangerous as `scalac` (Scala compiler) does not provide the safety net that Java does.

Throwing exceptions is not preferrable way of signaling errors in Scala code. Code should return `Try` or `Either` or signal errors via other error Monads.

In order to incorporate SDK into Scala code, callers will have to use `try { ... } catch { ... } ` blocks which is not only not "idiomatic Scala" but, most importantly, it pushes the responsibility of being aware of the errors to the caller: just from the type definition of a method, caller is not able to recognize which erorrs it has to handle so he/she has to read the code it's calling all the way down to define how to handle the exceptions. If he/she doesn't do it, it's risking runtime failures due to unhandled exceptions that will bring down the whole app.

Examples of exceptions being thrown that need changing to erorr types:
1. Methods are throwing exceptions when peers node is not available.
2. In method `send` (`HttpRPCService.scala:56`), `val response = post(...)` might throw an exception but it's not being caught (not part of `try { ... } catch { ...}` clause).
3. In `HttpRPCService.scala:send`, `JsonConverter.toJson` might throw an exception but it is not being handled.
4. Continuing on the above, all JSON manipulation methods can throw exceptions - `JsonConverter.fromJson`, `JsonConverter.toJson`.

**Strong Suggestion:** Refactor the code to use error-carrying types (like `Try`, `Either`, Error monads or special enums to distinguish various error cases).

**Severity:** HIGH

## Blocking calls

Even though there is a `sendAsync` variant implemented, it's never used. All methods in `CasperSdk` use `send` and are blocking. This may or may not impact the overall performance of the host project.

**Severity:** MEDIUM

## Usage of `Future.apply { ... }`

**file:** `HttpRPCService.scala`

`Future.apply` forks and runs the closure immediately on tha call-side. This may lead to unexpected results rather than `IO`-like libraries that describe the computation and leave the execution for later.

**severity:** LOW. `callAsync` is not used.

## Non-standard implementation of JSON (de)serialization.

Authors of Casper SDK chose to use Jackson as JSON (de)serialization library. It uses reflection which means it will fail in runtime rather than compile-time. Note that `fastparse-jackson` throws exceptions as well - which are not handled in the code.

**Severity:** MEDIUM.

**Suggestion:** Usage of native Scala libraries for JSON manipulation: circe, jsoniter-scala.

## Implementation of `Id` Monad.

For the purpose of returning `T` from API methods, the SDK authors implemented an `IdInstance` for `Monad`. Such instance of Monad is not "lawfull" - i.e. does not satisfy Monad laws.

**Severity:** LOW.

**Suggestion:** User proper `Monad` instance and _higher-kinded types_ in SDK API (`def foo[T](): F[T]`).