Open CMTaylor opened 9 years ago
It sounds like DirectAppExecCommand is a total copy-and-paste of AppCommand. In that case why not just use AppCommand?
As I say in the code comments for DirectAppExecCommand: // NOTE: This code was copied directly from AppCommand.m. The only difference in this command is that the // RequestRouter.m executes it on the HTTP thread rather than on the main UI thread.
There are certain situations in my app where executing a command on the main UI thread can cause a thread deadlock. In these situations I need to execute the command on “some other thread”, and the HTTP thread seemed easy and convenient. If you don’t think others need this kind of functionality, then you can skip this pull request. I’ve just been trying to get ALL my Frank modifications pushed into the main project on GitHub.
Thanks, Martin
From: Pete Hodgson [mailto:notifications@github.com] Sent: Monday, January 26, 2015 12:19 PM To: TestingWithFrank/Frank Cc: Taylor, Martin Subject: Re: [Frank] Direct app exec (#38)
It sounds like DirectAppExecCommand is a total copy-and-paste of AppCommand. In that case why not just use AppCommand?
— Reply to this email directly or view it on GitHubhttps://github.com/TestingWithFrank/Frank/pull/38#issuecomment-71509128.
What I was getting at is if it's the same implementation, why not just ditch DirectAppExecCommand
and do the following in FrankServer.m
:
[frankCommandRoute registerCommand:[[[AppCommand alloc] init]autorelease] withName:@"direct_app_exec"];
Either way, I'm not really happy with the hard-coding in RequestRouter
. If you can move the thread-jumping into AppCommand
and make it configurable via params (rather than on a separate API endpoint) then I'd be happy to pull it in.
The DirectAppExec command was copied directly from AppCommand.m. The only difference in this command is that the RequestRouter.m executes it on the HTTP thread rather than on the main UI thread. This is required in my application because running some commands on the UI thread can cause deadlocks.