Tell me more ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.
up vote 1 down vote favorite

Consider this function intended to kill all running instances of a subprocess:

killAllFoobars()
{
    pids = getRunningFoobars();

    foreach ( pids as p ) {
        killOneFoobar(p);
    }

    return TRUE;
}

Where should I check that in fact all Foobars were killed? Should killOneFoobar() check that its Foobar is now killed? Should killAllFoobars() check that there are no more running Foobars before returning TRUE (seems a bit disingenuous to return TRUE without checking). Should the function calling killAllFoobars() check?

Consider that the check is an expensive operation, so should not be done 'just whenever' but rather only when necessary.

share|improve this question
2  
You will never be sure that all the Foobars are killed (what if a new foobar is created after the check, but before returning from the function?). Just make sure that the killing functions work as intended. If you want to do more, you may loop forever, killing new processes as they are created. – coredump Jan 24 at 14:45
Thanks coredump, that is a good point and I think that it seals the fate of this question: I'll only check in killOneFoobar() that its own Foobar has been killed. Thus killAllFoobars() will (try to) kill only those Foobars running when the function was called. Please post your comment as an answer so that I might accept it. Thanks! – dotancohen Jan 24 at 14:56
I rewrote the comment as a question. Thanks too! – coredump Jan 24 at 15:07
Your return TRUE; is wrong anyway as it's the only return value, so it could be a void function instead. After getRunningFoobars() you have zero or more pids. And for each existing pid you should initiate the kill and wait for it's completion (fork or thread?). – ott-- Jan 24 at 15:22
@ott: It is psuedocode, the original pseudocode was in PHP style but someone added an inappropriate PHP tag. – dotancohen Jan 24 at 15:34

3 Answers

up vote 4 down vote accepted

You will never be sure that all the Foobars are killed (what if a new foobar is created after the check, but before returning from the function?). Just make sure that the killing functions work as intended. If you want to do more, you may loop forever, killing new processes as they are created.

share|improve this answer
Maybe he could as part of kill all foobars prevent new foobars from being created. And maybe return some token that could allow creating them again. – user470365 Jan 24 at 15:07
up vote 3 down vote

Do a count on $pids = getRunningFoobars(); after the foreach is finished to see if there are any that remain running. You can't return a 'true' just assuming. It may be an expensive check, but the check is needed before you confirm to the user of your function that it did succeed.

share|improve this answer
Thank you Jon. The question was "when to check" so I assume that you mean to say that the check should be at the end of killAllFoobars(), and not after the killAllFoobars() call in the calling function, nor at the end of killOneFoobar? – dotancohen Jan 20 at 10:15
You are welcome. I was saying to check just before returning 'true'. You want to automate as much within the function to ensure a smaller amount of code needed for your user, since it will need to be checked, whether by your code or your users. (This is assuming that checking fuBar is killed as you kill it is just as expensive as checking all at once (all at once would assume being quicker than a query each time) – Jon Jan 20 at 10:26
Did it help you at all? – Jon Jan 24 at 4:19
Yes, thank you Jon. – dotancohen Jan 24 at 15:35
up vote 1 down vote

If the expense of checking the kill of each foobar added up is equal to checking all of them, I would check in the deepest function, kill-one. If you were to reuse the kill-one function elsewhere it would be more accurate and useful. Also it consolidates your specific process logic into one function.

share|improve this answer
Ok a reasonable thing to do but then you lose the possibility to check "all Foobars were killed" so you will still have to do the overall check (what happens if a new foobar was started during the process?) – Mark Jan 24 at 13:21

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.