Re: why is applescript so slow???
Re: why is applescript so slow???
- Subject: Re: why is applescript so slow???
- From: Christopher Nebel <email@hidden>
- Date: Wed, 1 Dec 2004 15:24:44 -0800
On Nov 29, 2004, at 8:27 PM, halloolli wrote:
i have figured out a little script which duplicates all cards of the
addressbook and assigns these duplicates to a special group (thanks
heaps to Paul Berkowitz for the copy person part!).
in script editor this takes for 200 cards a couple of minutes! on a g4
imac!!! this can't be right, so what do i do wrong, or is applescript
really that slow??? i have attached the source code of my script.
There is no computer so fast, no language so efficient, that a bad
algorithm can't bring it to its knees. Ignoring your one algorithmic
problem, there are three key things about AppleScript that you're
missing:
1. Don't do searches when direct access methods exist. In particular,
you keep saying things like "first person whose name is p" when you
could say "person p" (or "person named p" if you're feeling finicky)
instead.
2. It's far more efficient to make AppleScript (or the application
you're talking to) do the loop than it is to write it yourself. (It's
also shorter and less error-prone.)
3. Each distinct command you send to an application has a minimum cost.
Find ways to bunch up operations; often, this will result in a shorter
and more resilient script as well as a faster one.
With those in mind, let's look at your script:
-- main
set mobileName to "_mobile"
set mobileGroup to my recreate_group(mobileName)
tell application "Address Book"
set refPeopleList to a reference to name of every person
repeat with pName in refPeopleList
set p to (first person whose name is pName)
if (name of every group of p) does not contain mobileName then
if not my dont_copy_this(p) then
add my copy_person(p) to mobileGroup
save addressbook
end if
end if
end repeat
end tell
First, the algorithmic problem: you're saving the address book after
every change. This is expensive. Moving it outside the loop cuts the
run time on my system by about 5x.
Next, the way you're doing the loop. The fact that you know about the
reference-to-list trick is perhaps admirable, but you've failed to
understand that it only applies to built-in lists. (Also, using it now
is what we'd call "premature optimization". In other words, fix the
big problems first, then go back and worry about the small ones if it's
still not fast enough.)
This...
set refPeopleList to a reference to name of every person
repeat with pName in refPeopleList
set p to (first person whose name is pName)
...is fairly pointless. What you're trying to do is loop over every
person, so why the translation from the names and back again? Just say
what you meant:
repeat with p in every person
(Some people will point out the reference-to-item oddity inherent in
the "repeat with p in a_list" form -- that is, that p isn't really the
value, it's a reference to it -- and suggest this instead:
repeat with i from 1 to (count people)
set p to person i
I consider this a matter of taste.)
It's possible to be even more efficient by observing that what you're
really trying to do is get all te people who are not in the "_mobile"
group. Ideally, we'd be able to express this in a "whose" clause, but
Address Book doesn't support that correctly (there's a bug filed on
that). You can, however, write it yourself using some simple
set-difference code (or use Smile; I believe it has set operation
commands). This is left as an exercise for the reader.
Moving on...
-- delete group with its cards and recreate it
on recreate_group(gName)
tell application "Address Book"
if ((name of every group) contains gName) then
set gFound to (the first group whose name is gName)
Again, don't use searches when you don't have to, and don't ask for
information you don't need. There's an "exists" verb, so use it:
if exists group gName then -- you can say "group gName exists", but I
prefer not to.
set gFound to group gName
Alternatively, you could just start trying to use group gName and deal
with the error if it doesn't exist:
try
get group gName
on error
-- if it doesn't exist, you'll wind up here.
end
set delList to {}
set allPeople to every person
script localScript
property refAllPeople : allPeople
end script
repeat with p in localScript's refAllPeople
set pGroupNames to name of every group of p
if pGroupNames contains (name of gFound) then
set end of delList to p
end if
end repeat
repeat with p in delList
delete p
end repeat
All of this code to delete the people in a group (I trimmed out the
confirmation dialog for clarity) can be replaced with four lines. If
you look at the dictionary, you'll see that "groups" contain "people"
as elements, so there's no need to go looking through every person.
Just say this:
get every person of group gName
repeat with i in the result
delete i
end
Runs a lot faster, too. (You ought to be able to say "delete every
person of group gName", and you can, but it only removes them from the
group and doesn't delete them. It's a bug.) You can get the count for
the confirmation dialog in much the same way:
count people of group gName
Incidentally, you may not need the 'if button returned of the result is
"OK"' bit -- if you hit the Cancel button, it'll throw a "user
canceled" error. If you don't catch it in a "try" block, your script
will simply stop, which is often the right thing to do.
-- check wether to copy this person over
on dont_copy_this(pers)
tell application "Address Book"
set w to words of (note of pers as string)
return w contains "nosync"
end tell
end dont_copy_this
This is about the best you can do here, since Address Book doesn't
directly support "word" elements of text, but if you're willing to
fudge slightly, you could say 'return note of pers contains "nosync"'.
(The "as string" works for sort of odd reasons. If a person hasn't got
any note, the "note" value is 'missing value'. AppleScript will coerce
this to the string "missing value" if you ask it to.)
tell target_p
repeat with i from 1 to (count allAddresses)
set addressProps to item i of allAddresses
tell addressProps to set targetAddressProps to
{label:label, street:street, city:city, state:state, zip:zip,
country:country, country code:country code}
make new address at end of addresses with properties
targetAddressProps
end repeat
Because of a couple of odd bugs/misdesigns, this is pretty much the way
to do it, but as of 10.3, you don't need the "at end of addresses" any
more. It occurs to me that since the point of these new cards is to
put on your phone, you may not care about, say, all the IM handles and
could just skip duplicating them.
_______________________________________________
Do not post admin requests to the list. They will be ignored.
Applescript-users mailing list (email@hidden)
Help/Unsubscribe/Update your Subscription:
This email sent to email@hidden