Yes, MUCH better 
Although there are still more things to consider.
First of all in a scenario when there are no PlayerControllers (yet), the loop body will never run. This could be an issue since you return ClosestPlayer which never had any value assigned explicitly. So it could contain garbage and upon trying to access anything through the pointer lead to a crash. Simply initialize it to NULL or nullptr and you should be safe (as long as you check whether your pointers are valid).
Something that shouldn’t really harm but is also not really useful in any way is the additional “if (!ActorItr) return;”. You’re already checking whether your iterator is valid in the loop condition (ActorItr;) and if it isn’t then you didn’t enter the loop body in the first place. And to answer your question in the comment in that line: Yes return will break out of the function and return the result (which you haven’t specified at this point) to the point where GetClosestPlayer has been called from, but you will not get to this point at all if there is no PlayerController found (see above).
EDIT (in response to mikepurvis):
The PlayerController isn’t what’s being casted here. It’s the PlayerController’s Pawn: ActorItr->GetPawn().
And something I forgot to mention: You should be able to simply replace TActorIterator<APlayerController> with TActorIterator<APawn> and this should allow you to iterate over all Pawns instead of all PlayerControllers.