Refactor native extension #4

d-unsed · 2016-09-21T20:28:09Z

Add Transform and TryTransform traits
Move Rust functions outside methods! macro
Change naming to match conventions

d-unsed · 2016-09-21T20:36:55Z

ext/case_transform/src/lib.rs


-class!(CaseTransform);
+trait Transform: Object {


Transform describes an interface for transformation. It can be implemented only to Ruby objects due to Object constraint.

Then we make type-specific implementations for

AnyObject (no-op)

RString

Symbol

Hash

Array

by implementing transorm methods.

d-unsed · 2016-09-21T20:52:53Z

ext/case_transform/src/lib.rs

-    fn transformSymbol(value: Symbol, transformMethod: &Fn(AnyObject) -> AnyObject) -> Symbol {
-        let transformed = transformMethod(value);
-        Symbol::new(transformed);
+trait TryTransform: Object {


TryTransform trait itself is needed only to define try_transform method on AnyObject. Rust does not allow to change the implementation of types that is not owned by current crate.

d-unsed · 2016-09-21T20:57:14Z

ext/case_transform/src/lib.rs

-        let transformed = transformMethod(value);
-        Symbol::new(transformed);
+trait TryTransform: Object {
+    fn try_transform<T>(&self,


try_transform<T> method:

Tries to convert current AnyObject to the type T.

:

If the conversion is successful it performs a transformation.

otherwise, returns an error from try_convert_to.

An example of usage of try_transform is

object.try_transform::<Hash>(key_transform) // if the object is a hash, try to do a hash transformation

The interesting part here is the constraints for T type: where T: VerifiedObject + Transform.

d-unsed · 2016-09-21T21:01:57Z

ext/case_transform/src/lib.rs

-            ValueType::Object => value
-        }
+fn transform(object: AnyObject, key_transform: &Fn(String) -> String) -> AnyObject {
+    let result = object.try_transform::<RString>(key_transform)


Try to transform the object as a RString
Otherwise try to transform the object as a Symbol
...
Otherwise try to transform the object as a AnyObject (it is any unkown class, do nothing)

d-unsed · 2016-09-21T21:03:12Z

ext/case_transform/src/lib.rs

-    fn toCamelCase(key: String) -> String { to_camel_case(to_snake_case(key.unwrap())) }
-    fn toDashedCase(key: String) -> String { to_kebab_case(to_snake_case(key.unwrap())) }
-    fn toSnakeCase(key: String) -> String { to_snake_case(key.unwrap()) }
+    result.unwrap()


We can safely unwrap here, because result has AnyObject type

- Add `Transform` and `TryTransform` traits - Move Rust functions outside `methods!` macro - Change naming to match conventions

d-unsed · 2016-09-21T21:08:00Z

ext/case_transform/src/lib.rs

-    fn dash(value: AnyObject) -> AnyObject { transform(value.unwrap().to_any_object(), &dash, &toDashedCase) }
-    fn underscore(value: AnyObject) -> AnyObject { transform(value.unwrap(), &underscore, &toSnakeCase) }
-    fn unaltered(value: AnyObject) -> AnyObject { value.unwrap().to_any_object() }
+    fn camel(object: AnyObject) -> AnyObject { transform(value.unwrap(), &to_pascal_case) }


And we can safely unwrap in this methods, because objects are AnyObjects

d-unsed · 2016-09-21T21:10:14Z

ext/case_transform/src/lib.rs

-    fn toSnakeCase(key: String) -> String { to_snake_case(key.unwrap()) }
+class!(CaseTransform);
+
+methods! (


All the functions which are not used directly in the CaseTransform class are moved outside methods!

this makes sense!

d-unsed · 2016-09-21T21:10:56Z

That's what I meant in d-unsed/ruru#35 :)

I hope it helps!

# Running:

....

Finished in 2.039933s, 1.9608 runs/s, 28.9225 assertions/s.

NullVoxPopuli · 2016-09-21T21:19:15Z

ext/case_transform/src/lib.rs

-
-use ruru::{Class, Object, RString, Hash, Array, Symbol, AnyObject, VM};
-use ruru::types::ValueType;
+use inflector::cases::{camelcase, classcase, kebabcase, snakecase};


this is handy!

is there a way to import the methods into the 'global' namespace of the file? so you don't have to do classcase::to_class_case?

NullVoxPopuli · 2016-09-21T21:19:54Z

ext/case_transform/src/lib.rs


-class!(CaseTransform);


why was this removed? I saw it one one of the example repos, though I don't know what its purpose was

The line was moved to the bottom of the file

NullVoxPopuli · 2016-09-21T21:21:43Z

ext/case_transform/src/lib.rs

-methods! (
-    CaseTransform,
-    itself,
+trait Transform: Object {


I need to read up on traits and impls

for myself:

https://doc.rust-lang.org/book/traits.html

https://doc.rust-lang.org/book/generics.html looks like impl's are just implementations of a generic struct?

NullVoxPopuli · 2016-09-21T21:23:50Z

ext/case_transform/src/lib.rs

-    fn transformSymbol(value: Symbol, transformMethod: &Fn(AnyObject) -> AnyObject) -> Symbol {
-        let transformed = transformMethod(value);
-        Symbol::new(transformed);
+        result.into_iter()


arrays are iterable by default?

edit: not*

They kinda are iterable, but after explicit conversion to iterators :) Same for slices, vectors etc

NullVoxPopuli · 2016-09-21T21:27:33Z

ext/case_transform/src/lib.rs

-    fn toCamelCase(key: String) -> String { to_camel_case(to_snake_case(key.unwrap())) }
-    fn toDashedCase(key: String) -> String { to_kebab_case(to_snake_case(key.unwrap())) }
-    fn toSnakeCase(key: String) -> String { to_snake_case(key.unwrap()) }
+class!(CaseTransform);


here it is!

NullVoxPopuli · 2016-09-21T21:34:11Z

@d-unseductable this helps a ton! thanks so much!

NullVoxPopuli · 2016-09-21T21:34:42Z

I have to wait till I get to my personal laptop to try this out, so I have consistent hardware for my benchmarking :-)

d-unsed · 2016-09-21T21:47:02Z

You're welcome! Please let me know when you get the results

NullVoxPopuli · 2016-09-21T21:48:52Z

Just for some background here is the comparison between pure ruby, and my last successful compile: rails-api/active_model_serializers#1928 (comment)

It actually significantly hurt performance. haha.

NullVoxPopuli · 2016-09-21T22:58:17Z

rails-api/active_model_serializers#1928 (comment)

So, slightly faster... but still am not sure what the deal is with the slowness. Still, I appreciate you teaching me rust!

NullVoxPopuli · 2016-09-21T23:34:22Z

I guess what could be next is seeing if fiddle is slowing me down.

d-unsed force-pushed the transform_trait branch from 803b258 to 86b302e Compare September 21, 2016 20:33

d-unsed commented Sep 21, 2016

View reviewed changes

Refactor native extension

f7dc641

- Add `Transform` and `TryTransform` traits - Move Rust functions outside `methods!` macro - Change naming to match conventions

d-unsed force-pushed the transform_trait branch from 86b302e to f7dc641 Compare September 21, 2016 21:05

d-unsed commented Sep 21, 2016

View reviewed changes

NullVoxPopuli reviewed Sep 21, 2016

View reviewed changes

NullVoxPopuli merged commit 8056028 into rails-api:ruru-rust-speed-boost Sep 21, 2016

d-unsed deleted the transform_trait branch September 22, 2016 06:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor native extension #4

Refactor native extension #4

d-unsed commented Sep 21, 2016

d-unsed Sep 21, 2016 •

edited

Loading

d-unsed Sep 21, 2016

d-unsed Sep 21, 2016

d-unsed Sep 21, 2016

d-unsed Sep 21, 2016

d-unsed Sep 21, 2016

NullVoxPopuli Sep 21, 2016

d-unsed Sep 21, 2016

NullVoxPopuli Sep 21, 2016

d-unsed commented Sep 21, 2016 •

edited

Loading

NullVoxPopuli Sep 21, 2016

NullVoxPopuli Sep 21, 2016

NullVoxPopuli Sep 21, 2016

d-unsed Sep 21, 2016

NullVoxPopuli Sep 21, 2016

NullVoxPopuli Sep 21, 2016

NullVoxPopuli Sep 21, 2016 •

edited

Loading

d-unsed Sep 21, 2016 •

edited

Loading

NullVoxPopuli Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016

d-unsed commented Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016


		class!(CaseTransform);

Refactor native extension #4

Refactor native extension #4

Conversation

d-unsed commented Sep 21, 2016

d-unsed Sep 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d-unsed commented Sep 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NullVoxPopuli Sep 21, 2016 • edited Loading

Choose a reason for hiding this comment

d-unsed Sep 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NullVoxPopuli commented Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016

d-unsed commented Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016

NullVoxPopuli commented Sep 21, 2016

d-unsed Sep 21, 2016 •

edited

Loading

d-unsed commented Sep 21, 2016 •

edited

Loading

NullVoxPopuli Sep 21, 2016 •

edited

Loading

d-unsed Sep 21, 2016 •

edited

Loading