Looking for critiques

Scripting Forums AppleScript | Mac OS X
,
#1

OK so I have this script here that works perfectly for me, as far I know. I am just wondering if there are any improvements that can be made such as subroutines and etc.

Here is the script and I am looking forward to your critiques.

–the administrator password
set the_pass to “*********”
–the whole date including time
set the_date to (current date) as text
–path to nmap log
set file_open to “STURM:Users:iconan:nmaplog.txt”
–set some search information
set search_text to the_date
–path to nmap log as alias
set the_file to “STURM:Users:iconan:nmaplog.txt” as alias
–another use for the current date but not as text
set the_2date to current date
–the whole date not including time
copy word 2 of ((current date) as text) & " " & word 3 of ((current date) as text) ¬
& " " & word 4 of ((current date) as text) to the_Filedate

–get the creation date of nmap log
copy (info for the_file) to fileInfoRecord
tell fileInfoRecord
set the_Credate to the creation date
end tell

–get the difference between execution date and creation date of nmap log
set the_diff to the_2date - the_Credate

–if the log is older than one week rename it including date of execution of script
if the_diff is greater than 604800 then
tell application “Finder”
set the_name to name of the_file
set name of the_file to the_name & the_Filedate
end tell
end if

–do shell scripts to gather information on computers and router in network
set _nmap to do shell script ¬
“sudo /sw/bin/nmap -sT -v -O 192.168.1.100” password the_pass with administrator privileges

set _nmap2 to do shell script ¬
“sudo /sw/bin/nmap -sT -v -O 192.168.1.101” password the_pass with administrator privileges

set _nmap4 to do shell script ¬
“sudo /sw/bin/nmap -sT -v -O 192.168.1.102” password the_pass with administrator privileges

set _nmap5 to do shell script ¬
“sudo /sw/bin/nmap -sT -v -O 192.168.1.103” password the_pass with administrator privileges

set _nmap3 to do shell script ¬
“sudo /sw/bin/nmap -sT -v -O 192.168.1.1” password the_pass with administrator privileges

–prepare the days log file
set _nmaplog to return & return & the_date & return & return & _nmap & return & return ¬
& _nmap2 & return & return & _nmap3 & return & return & _nmap4 & return & return ¬
& _nmap5 & return & return

–write the log and create log file if it does not exist
set the_log to open for access file (((path to current user folder) as string) & “nmaplog.txt”) with write permission
try
write _nmaplog to the_log starting at eof
close access the_log
on error
try
close access the_log
end try
end try

–open log and look for date of script execution and change color and size
tell application “Tex-Edit Plus”
activate
open {file_open} as alias
select contents of window 1
search selection looking for search_text
set size of contents of selection to 18
set color of contents of selection to blue
end tell

–open log and look for IP addresses on local network beginnning with “192” and change size and color
tell window 1 of application “Tex-Edit Plus”
select insertion point before paragraph 1 of window 1
repeat
search looking for “192” with searching from cursor
if result is not false then
set first_word to get word offset of selection
set last_word to ((get word offset of selection) + 3)
select words first_word thru last_word of window 1
set size of contents of selection to 16
set color of selection to red
else
exit repeat
end if
end repeat
end tell

–quit Tex-Edit" application
tell application “Tex-Edit Plus”
quit saving yes
end tell

Thanks…

#2

This seems fine. I can’t see any need for handlers (subroutines). Just a few efficiency points regarding the vanilla and Finder parts of your script:

You’ve used the ‘current date’ command five times here. You only need to call it once and then work with the result:

-- the current date as a date
set the_2date to current date
-- the current date as string, including the time
set the_date to the_2date as text
-- the current date as string, excluding the time and weekday
set the_Filedate to text from word 2 to word 4 of the_date

This should be OK on your machine. AppleScript coerces dates to text according to the format set up in the user’s preferences, but most users will have a one-word weekday, followed by day, month, and year (in some order), and the time (in some format).

You’ve also set up an explicit path string and alias to your file - which is fine. But when you open the file for writing later on, you work it all out again using ‘path to’; and when you then open the file in TextEdit, you put the path string into a list and coerce it to alias. You do have a reason for this, since the script renames an old file of that name and then creates a new one. It might be less confusing for a casual reader if you only set up the path string here and coerce it to alias at the appropriate points in the script. It’s a good idea (in my personal opinion) to use the ‘path to’ approach anyway:

--path to nmap log
set file_path to (path to current user folder as string) & "nmaplog.txt"

Notice that ‘path to’ can take ‘as string’ (or ‘as text’) as an optional parameter, so there are two ways of getting a path string from it:

-- 'path to' with parameter:
(path to current user folder as string)

-- 'path to' followed by coercion:
((path to current user folder) as string)

In the first case, the ‘path to’ command itself returns the string. In the second case, ‘path to’ returns an alias which is then coerced to string by the AppleScript language. The end result is exactly the same, but the first way is more efficient.

You’ve sensibly used ‘info for’ rather than the Finder to get the creation date of the existing file as this is usually faster. It’s also faster at getting the name of the file - though in fact, since you know the name already, you could simply write it into the script. In theory, the first time you run the script, the file won’t exist, so a ‘try’ block would be a good idea.

There’s no need to use ‘copy’ in the line that sets fileInfoRecord. Since there’s no danger of errors from data sharing, ‘set’ would do perfectly well. Some people like to use ‘copy’ all the time, but the two commands are not the same. ‘Set’ sets something to a value or object; ‘copy’ duplicates that value or object first and then sets something to the duplicate - which obviously takes more memory and is more work.

AppleScript has a predefined constant ‘weeks’, which has a value of 604800. You can use this instead of 604800 in a script. It’s not more efficient, but it’s easier to read and saves you having to work out the number of seconds in a week. There are also the constants ‘minutes’, ‘hours’, and ‘days’. (The plural endings are so that they look good in arithmetical expressions such as ‘(current date) + 4 * weeks’. They also avoid confusion with the date property ‘day’.)

--get the creation date of nmap log
try -- the next line errors if the file doesn't exist
  set the_file to file_path as alias
  set fileInfoRecord to (info for the_file)
  tell fileInfoRecord
    set the_Credate to its creation date
    set the_name to its name
  end tell

  --get the difference between execution date and creation date of nmap log
  set the_diff to the_2date - the_Credate

  --if the log is older than one week rename it including date of execution of script
  if the_diff is greater than weeks then
    tell application "Finder"
      set name of the_file to the_name & the_Filedate
    end tell
  end if
end try

There are one or two points were you’ve harmlessly set variables you don’t need. ‘the_diff’ is a case in point. You don’t need to use the value again, so you could save yourself a line of script and the bother of thinking up a variable name:

  -- if the log is older than one week rename it including date of execution of script
  if (the_2date - the_Credate) is greater than weeks then
     -- etc

I hope this has been of interest. :slight_smile:

#3

Thanks very much for the pointers. I have never tried to do this much with Applescript before. Just small things usually.

#4

Actually, I disagree with Nigel’s analysis of the line:

copy word 2 of ((current date) as text) & " " & word 3 of ((current date) as text) ¬ 
         & " " & word 4 of ((current date) as text) to the_Filedate

This is totally non-portable. It may work on your machine today, but not if you change your preferences, not on many other people’s systems, and almost certainly not on any non-US users’ system.

You are much better off using the date properties:

set theDate to (current date)
set the_Filedate to (month of theDate) & " " & (day of theDate) & " " & (year of theDate) as string

This will work no matter what the user’s preferences, and no matter what language setting they use.

That said, using spaces in filenames in conjunction with unix apps is never a good idea. You get into all kinds of issues with having to escape and/or quote the filename. You’d be much better off using some other delimiter such as an underscore.

#5

It’s not reliably portable, but it works on my UK sytem.

This will impose US date format, no matter what the user’s preferences, and no matter what language setting they use - which is not quite the same thing.

#6

But isn’t that better than ending up with “June 2003 Monday” which you’re going to end up with (at least in some translated form) when you rely purely on positional words based on some unknown date format setting on the users’ system.

#7

Neither is satisfactory as an example of portability.

C. Mullins’s text-extraction approach “works” with all the OS X long-date presets except those for Brazil and Portugal (though if they’re all as rare in their own countries as the “UK” one is in mine, most people will be using custom formats). We’ve rightly pointed out to him/her that it’s not reliably portable - that for other users it may produce nonsense results - but he/she hasn’t said whether or not the script is to be used by others. The best we can do is to point things out and let people make their own decisions based on their knowledge of the intended environment and their own scripting ability.

Personally, if I wanted to put a date into a file name, I’d use a short date in yyyymmdd format. This would make sense to most people and would sort well in Finder displays.

set the_2date to current date
set the_file to ((path to current user folder as string) & "nmaplog.txt") as alias
-- blah blah blah

tell application "Finder"
  set the_file's name to "nmaplog.txt_" & my yyyymmdd(the_2date)
end tell
-- etc.


on yyyymmdd(theDate)
  set {year:yyyy, day:dd} to theDate
  
  copy theDate to b
  set b's month to January
  set mm to (b - 2500000 - theDate) div -2500000
  
  tell (yyyy * 10000 + mm * 100 + dd) as string
    return text 1 thru 4 & "_" & text 5 thru 6 & "_" & text 7 thru 8
  end tell
end yyyymmdd
#8

Actually I am a he if that matters. I do not care if others use it I was just doing it for personal use. Other than that I would like to thank all the people that replied and I am changing my script around to see how it works. I am working on an advanced version right now.

Also, I would think that if others wanted to use my script and it did not work as planned they would either have the know how to fix it or they would come and ask questions just like I did.

Again, Thank You…