Any suggestions for improvement
A general suggestion:
You have many functions checking whether a variable is not equal to nil, such as:
(if (/= fd nil) ;正常OpenAny value stored in a variable that is not nil will "satisfy" an (if) or (while) test. It is not necessary to test whether something is "not nil," but only whether it exists. So all you need is:
(if fd ;正常Open
....
(while in_data
....
(if in_data
....
;;;;;;;;;;;;;;
....
(while pos
....
(if pos
....You can shorten some things. You have it opening the file and then checking whether it's there, in two steps, but you could just have it check whether the file opens, in one step. Then instead of a (while) loop that checks whether in_data is not nil, then reads another line, then checks whether it's still not nil, you can just have the (while) loop simply check whether there's another line to read, again saving a few steps. And in a (while) loop, the (progn) "wrapper" is not necessary as it is when you want to do more than one thing as an (if) function's 'then' argument.
....
(if (setq fd (open r_file "r")) ;正常Open
(progn
(setq in_data (read-line fd)) ;Header Line read
(while (setq in_data (read-line fd)) ;Read one line [returns nil at end of file]
(setq lst (_CSVtoList in_data))
(princ lst)
(princ "\n")
); while
(close fd)
); progn
(princ "Bat File name")
); if
....Similarly, with the pos variable, you have it checking whether it's still there, then looking for a comma again, then checking whether it found one, but you can have it simply check whether there's another comma to find, all in one step:
....
(while (setq pos (vl-string-search "," CSV)); still another comma?
(setq result (append result (list (substr CSV 1 pos))))
(setq CSV (substr CSV (+ pos 2) len))
(setq len (strlen CSV))
); while
(setq result (append result (list CSV))) ;pos == nil CSV last Item
....
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;CSV file Read
(defun c:Read_File (/ r_file fd in_data lst)
(if ;; Always test if the file exists | ALWAYSSSSSSSS
(setq r_file
(findfile
(strcat
(getenv "APPDATA") ;<- Reads workstations %appdata% folder
"\\Autodesk\\Support\\AcadMenu\\UnfoldMenu\\Material.csv"
)
)
)
(progn
(setq fd (open r_file "r")) ;<-then this line will be able to open a guaranteed existing file
;; (/= fd nil) ;<-Since the first line is valid you dont need this test
(setq in_data (read-line fd)) ;<-I'm guessing you did this to exclude the header value
(while
(setq in_data (read-line fd)) ;<- Continuing on the next line | read-line will evaluate to nil when
; it hits the end of file (EOF) and effectively stops the loop
; on the other hand, a blank line will give you "" [ not a null value ]
;;(if (/= in_data nul) ; <- Dont need it as explain above, BTW you mispelled "null"
;<- The string value maybe a"blank" --> ""
;<- or even a tab or white space, what you need is to check for the
;<- the validity of the contents, say in this example this character ","
(if (setq f
(Vl-string-search
"," in_data)) ;<- search for the valid character and save the value, that will be
;<- your first pos value instead of 0
(progn
(setq lst (_CSVtoList in_data f)) ;<- use the fucntion cons to buidld the list
(print lst) ; (setq lst (cons (_CSVtoList in_data f) lst))
); progn
);if comma is found
); while read-line is not nil
(close fd)
)
(princ "Bad File name")
)
)
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;;CSVtoList
(defun _CSVtoList (CSV pos / len pos result)
(While pos ; pos value already defined as an argument
(setq result (append result (list (substr CSV 1 pos))))
(setq CSV (substr CSV (+ pos 2)))
(setq pos (vl-string-search "," CSV))
)
(append result (list CSV))
)
More on the CSVtoList function
(defun _CSVtoList (CSV / len pos result)
;(setq len (strlen CSV)) ;<- dont need it | continue reading below
;(setq pos 0) ;<- dont need it
;(setq result nil) ;<- dont need it | declared as local variable
(while
;(/= pos nil) ;<- dont need it | as you already assign 0 at the beginning
(setq pos
(vl-string-search "," CSV)) ;<- this is enough for evaluation for both (/= nil pos)
;(if (/= nil pos) ;<- dont need it
;(progn ;pos != ni ; You dont need a wrapper inside the while loop
(setq result (append result (list (substr CSV 1 pos))))
; Note that this two lines here produce the same result
; (setq CSV (substr CSV (+ pos 2) len))
; (setq CSV (substr CSV (+ pos 2)))
; hence the removal of (setq len (strlen CSV))
(setq CSV (substr CSV (+ pos 2) ))
;(setq len (strlen CSV));<- dont need it | from the statement above
)
;(setq result ; Since this is outside the loop it will be the last statement
(append result (list CSV)) ; This is enough to produce the same result
;) ;<- dont need it
;) ;<- dont need it
;result ;<- dont need it
)
HTH
