search

michalmuskala / jason

A blazing fast JSON parser and generator in pure Elixir.
Other
1.6k stars 170 forks source link

Delete unused Jason.Codegen.jump_table_case/4 #108

Closed QuinnWilton closed 4 years ago

QuinnWilton commented 4 years ago

While reading through the code I noticed that this macro was unused.

michalmuskala commented 4 years ago

Thank you! :heart:

QuinnWilton commented 4 years ago

Edit: I actually just went and made this change, and it seems like the tests do in fact fail, so the code is needed. Sorry for the false alarm :) I hadn't yet gone through the encoding logic, and didn't realize Codegen.jump_table/2 was called directly.

Thank you for the quick merge! While tracing through the code last night, I noticed a few other bits of code that don't seem to be used, but I was hesitant to delete them in case you weren't interested in that kind of cleanup PR.

For example, as far as I can tell, this clause is never hit, since ranges are only defined using charlists: https://github.com/michalmuskala/jason/blob/master/lib/codegen.ex#L96

Would it be helpful if I also got rid of code like that too?

michalmuskala commented 4 years ago

I believe it's used in here - it generates a list of single-integer elements

https://github.com/michalmuskala/jason/blob/91a4eaf5b3b35e0dc7c2efc408dfdac7fcd2a7cd/lib/encode.ex#L261

But yeah, if there's any unused code, it should definitely go.

QuinnWilton commented 4 years ago

Yup, you're right. I missed the direct calls to Codegen.jump_table/2. Will do, thanks for getting back to me!