surrealdb / surrealdb.wasm

A WebAssembly engine for the SurrealDB JavaScript SDK
https://surrealdb.com
Apache License 2.0
115 stars 19 forks source link

`convert.rs` - `else if` chain to `match` #1

Closed ryanrussell closed 2 years ago

ryanrussell commented 2 years ago

Issue: Big O

Something about this in a rust + wasm repo doesn't feel right :)

Solution:

Switch else if chain to match

Too early for PR's on this repo? Do not feel like you need to wait for me on updating/changing this :)

I may be jumping the gun on code audits here...

impl Convert<Value> for JsValue {
    fn convert(self) -> Value {
        if self.is_null() {
            Value::Null
        } else if self.is_undefined() {
        ...
tobiemh commented 2 years ago

Hey @ryanrussell suggest ahead. Not too early! I wasn't sure of a better way of doing it according to the API, as the values weren't all necessarily enums which were able to be matched on!

ryanrussell commented 2 years ago

Will take a look. Love the direction you're taking surrealdb... About time someone tackled this well in rust :)

ryanrussell commented 2 years ago

@tobiemh -- thanks for explaining the reasoning on this one :+1:

Since there are types in the match, it looks like to refactor the if/else if code block, one option would be to create a trait, then implement a function for each type like this SO.

Frankly, this feels a bit pedantic and I'm not sure the improvement delta is worth it or that this is even the correct solution based on future intent... at this juncture of SurrealDB, there are likely much brighter fires with bigger upside.

Going to close this out for now. Perhaps when things are further down the road this is worth revisiting...