mattn / anko

Scriptable interpreter written in golang
http://play-anko.appspot.com/
MIT License
1.45k stars 120 forks source link

Move logic outside of main switch #326

Closed alaingilbert closed 4 years ago

alaingilbert commented 4 years ago

I have a fork of this project. One of the thing I did was to extract the logic out of the switch. I find it easier to read and follow the code. Are you interested by this change ?

codecov-io commented 4 years ago

Codecov Report

Merging #326 into master will increase coverage by 0.16%. The diff coverage is 96.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
+ Coverage   87.42%   87.58%   +0.16%     
==========================================
  Files          15       15              
  Lines        2663     2699      +36     
==========================================
+ Hits         2328     2364      +36     
  Misses        216      216              
  Partials      119      119
Impacted Files Coverage Δ
vm/vmStmt.go 96.44% <96.02%> (+0.24%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5f413ab...7e986d4. Read the comment docs.

MichaelS11 commented 4 years ago

Not really seeing how it makes it easier to read or follow the code.

alaingilbert commented 4 years ago

Well, having small functions that you can "goto definition" and go back. Instead of a single function with a thousand lines...

I can see right away what are all the "statements" types that are handled by the runSingleStmt function just by looking at the switch. (instead of having to scroll through it, not knowing what is where)

MichaelS11 commented 4 years ago

Ok, I get what you are saying.

Note that all the statements are in https://github.com/mattn/anko/blob/master/ast/stmt.go already, so not that hard to see what statements are handled.

I am not partiularly for this change.

Plus I am not sure if the commit is clean. At least Github online diff is showing a lot of other diffrences.

alaingilbert commented 4 years ago

The main point is code navigation & comprehension.

The git diff is indeed quite ugly image

But the only thing I did was:

But yeah, no big deal, if you don't want this :)

MichaelS11 commented 4 years ago

Thanks for looking into this.

@mattn has the final say.