Conversation
lib/debug/session.rb
Outdated
| } | ||
| @tp_load_script.enable | ||
|
|
||
| @thread_stopper = begin |
There was a problem hiding this comment.
I don't see any reason to lazy-evaluate this value and assign it in the constructor seems more intuitive to understand? (and can save some additional checks)
There was a problem hiding this comment.
please revert this change. lazyness is not important (so you can call thread_stopper here) but complex logic should be located near lines.
| setup_threads | ||
| thc = thread_client Thread.current | ||
| thc.is_management | ||
| thc = get_thread_client Thread.current |
There was a problem hiding this comment.
thread_client feels like a variable instead of a method call. and I think adding the get_ prefix matches other method names like ask_thread_client.
| end | ||
|
|
||
| def thread_switch n | ||
| def switch_thread n |
There was a problem hiding this comment.
most of other methods are named in the Verb-Noun fashion.
There was a problem hiding this comment.
it intends: thread.switch
but okay to change.
| end | ||
|
|
||
| private def thread_client_create th | ||
| private def create_thread_client th |
There was a problem hiding this comment.
most of other methods are named in the Verb-Noun fashion.
| @th_clients[th] = ThreadClient.new((@tc_id += 1), @q_evt, Queue.new, th) | ||
| end | ||
|
|
||
| private def ask_thread_client th = Thread.current |
There was a problem hiding this comment.
this default value is never used.
st0012
force-pushed
the
refactor-thread-management
branch
from
9a21105 to
97134a8
Compare
|
|
||
| def deactivate | ||
| thread_client.deactivate | ||
| @thread_stopper.disable if @thread_stopper |
There was a problem hiding this comment.
@ko1 I still think initializing @thread_stopper at the beginning and get rid of this check is important, so I changed my approach a bit instead of revert it. can you check it again?
2 participants
I was catching up all the recent changes and I think some small changes can make the code easier to understand.