HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Should a piece of code only ever called once be a separate method?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
oncecalledmethodpieceseparateshouldcodeeveronly

Problem

I'm curious when a piece of code should be its own method or just left alone. This came up when I was creating a "work item" from the thread pool. When passing the argument to the "WaitCallback" object, I would argue that if the code that will run will ONLY run when this thread is being run, then it's OK to wrap the code inside of an Action delegate. If other places need to run this same code, then I understand separating it into a new method.

So, for example, what's better practice?

This --

ThreadPool.QueueUserWorkItem(new WaitCallback((state) =>
{
    Contract.SaveContract(Customer);
     pushResultNotification = DoPushContact();
      MyDispatcher.BeginInvoke((Action)(() => actionNotification.CompleteAction()), null);
}));


Or this --

ThreadPool.QueueUserWorkItem(new WaitCallback(DoStuff));

private void DoStuff(object state)
{
    Contract.SaveContract(Customer);
    pushResultNotification = DoPushContactToGp();
    MyDispatcher.BeginInvoke((Action)(() => actionNotification.CompleteAction()), null);
}

Solution

I think the answer here is no, it shouldn't be a separate method, as long as the piece of code in question is short. That way, when you read the code, you immediately see what the code does, you don't have to go elsewhere for that. But if the piece of code was too long, it would make the whole method less readable, so in that case, I think you should put it into a separate method.

Few more notes:

You don't need the new WaitCallback() in either case, the compiler can create the delegate automatically.

Also, it might make sense to use Task instead of ThreadPool, assuming you're on .Net 4+.

Context

StackExchange Code Review Q#17893, answer score: 6

Revisions (0)

No revisions yet.