Can someone double check my work?

[BatMan22]

So I wrote this program and I really need someone to check my work because if I screw this up, I get fired. These are legal documents and the federal government will have my tush and my companies tush if I mess anything up. So my program will monitor a local directory for any new files, and if any new files are made it will rename them to the YYMMDDXX order in the order that they are created... Where XX starts at 01 and goes to 99, there should be no more then 99 files in a given day.  Just hoping you guys could check the code and see if anything COULD go wrong? I just don't want files ever getting deleted and/or renamed incorrectly. Also is there a way to prevent this program from working if someone copies files into the directory VS creating files in the directory.. aka.. This program shouldn't rename any files that are moved/copied into the directory all at once. The program that is creating files in this directory will only create one at a time.

 

Maily the things that scare me are the _WinAPI_CreateFileEx, _WinAPI_CreateBuffer, _WinAPI_ReadDirectoryChanges commands. I copied those directly from someone else on the forums and don't REALLY understand what's going on beyond what F1 is telling me. 

Thanks all

expandcollapse popup

#Region ;**** Directives created by AutoIt3Wrapper_GUI ****
#AutoIt3Wrapper_Run_Tidy=y
#EndRegion ;**** Directives created by AutoIt3Wrapper_GUI ****
#include <APIFilesConstants.au3>
#include <Array.au3>
#include <MsgBoxConstants.au3>
#include <WinAPIDiag.au3>
#include <WinAPIFiles.au3>
#include <WinAPISys.au3>
#include <Misc.au3>


Global $g_sPath = @ScriptDir ; Directory to monitor
Global $WorkingDir = @ScriptDir & "\" ; Directory to monitor AND a dash to make life easier.
Local $Running

$Running = _Singleton("DirectoryWatch.exe", 1)
If $Running = 0 Then
    MsgBox(0, "ERROR!", "Only ONE instance of this program may be run at ONCE, Restart the computer or force close the other instances!")
    Exit
EndIf



Local $hDirectory = _WinAPI_CreateFileEx($g_sPath, $OPEN_EXISTING, $FILE_LIST_DIRECTORY, BitOR($FILE_SHARE_READ, $FILE_SHARE_WRITE), $FILE_FLAG_BACKUP_SEMANTICS)
If @error Then
    _WinAPI_ShowLastError('', 1)
EndIf

Local $pBuffer = _WinAPI_CreateBuffer(8388608)
Local $aData

While 1
    $aData = _WinAPI_ReadDirectoryChanges($hDirectory, BitOR($FILE_NOTIFY_CHANGE_FILE_NAME, $FILE_NOTIFY_CHANGE_DIR_NAME), $pBuffer, 8388608, 1)
    If Not @error Then
        ConsoleWrite("Changed Filename Is : " & $aData[0][0] & @CRLF & "Function Performed Is: " & $aData[0][1] & @CRLF)
        Local $delrange = "0"
        _ArrayDelete($aData, $delrange)
        ConsoleWrite("Changed Filename Is : " & $aData[0][0] & @CRLF & "Function Performed Is: " & $aData[0][1] & @CRLF)
        Local $filename = $aData[0][0]
        Local $functionpreformed = $aData[0][1]
        If $functionpreformed = 1 Then
            RenameMe($filename)
        EndIf
    Else
        _WinAPI_ShowLastError('', 1)
    EndIf
WEnd

Func RenameMe($NameBefore)
    Local $FileDate, $increment = "1", $YYformat
    $FileDate = FileGetTime($NameBefore, 1)
    $YYformat = $FileDate[0] - 2000
    If $YYformat < 18 Then
        MsgBox(0, 0, "Jackass, you're COPYING files into this directory that were created before or on the year 2017 and the program is renaming them! This program is only meant for new files made in the directory!")
    EndIf

    While FileExists($WorkingDir & $YYformat & $FileDate[1] & $FileDate[2] & StringFormat("%02d", $increment)) = 1
        $increment = $increment + 1
        If $increment = 100 Then
            MsgBox(0, "ERROR!", "Why are there MORE THEN 100 FILES run today! File Renaming has been STOPPED! THIS IS NOT POSSIBLE!")
            Exit
        EndIf
        Sleep(50) ; prevent errors?
    WEnd
    FileMove($WorkingDir & $NameBefore, $WorkingDir & $YYformat & $FileDate[1] & $FileDate[2] & StringFormat("%02d", $increment))
    Sleep(500)
    $increment = 0 ;reset increment just in fucking case.
EndFunc   ;==>RenameMe

 

Edited January 17, 2018 by BatMan22
Added Singleton function

[spudw2k]

First, some opinions/suggestions--my apologies if the tone comes off harsh:

A. Don't do anything that could be risky, whether for you personally or your employer.   Test, test, test--using fake/example data/files instead of real data.

B. There's some "interesting" language choices in your script you might want to change from a professionalism stand-point.

C. Why are you using functions that you don't fully understand if you are concerned about risk?

 

Secondly, some code analysis input and questions:

A. The only thing I am seeing as potentially "destructive" is the FileMove operation.  Could you employ FileCopy instead as to not alter the integrity or risk loss of the original file?   Why is it important to rename the files in your desired manner?  Is there a chance that files would be overwritten due to not unique file names, or is file overwriting a common occurance and you want to maintain "versioning"?

B. One thing I am a little concerned about, but don't know what repercussions could occur is the call to _WinAPI_CreateFileEx doesn't have a follow-up closing function to clean up the handle (_WinAPI_CloseHandle).  I notice that function is missing from the _WinAPI_ReadDirectoryChanges function as well, but it is referenced in the help article for _CreateFileEx; so I don't know how important it is.  Having said that, nothing intrinsic about the functions is jumping out at me as being sensitive to major risk.  

 

I'd encourage you keep testing.  Maybe create another script to produce mock files around the intervals you expect files to be made in production and let it run for as long as you'd expect your monitoring/renaming script to run unattended.

[BatMan22]

1 hour ago, spudw2k said:

First, some opinions/suggestions--my apologies if the tone comes off harsh:

A. Don't do anything that could be risky, whether for you personally or your employer.   Test, test, test--using fake/example data/files instead of real data.

B. There's some "interesting" language choices in your script you might want to change from a professionalism stand-point.

C. Why are you using functions that you don't fully understand if you are concerned about risk?

 

Secondly, some code analysis input and questions:

A. The only thing I am seeing as potentially "destructive" is the FileMove operation.  Could you employ FileCopy instead as to not alter the integrity or risk loss of the original file?   Why is it important to rename the files in your desired manner?  Is there a chance that files would be overwritten due to not unique file names, or is file overwriting a common occurance and you want to maintain "versioning"?

B. One thing I am a little concerned about, but don't know what repercussions could occur is the call to _WinAPI_CreateFileEx doesn't have a follow-up closing function to clean up the handle (_WinAPI_CloseHandle).  I notice that function is missing from the _WinAPI_ReadDirectoryChanges function as well, but it is referenced in the help article for _CreateFileEx; so I don't know how important it is.  Having said that, nothing intrinsic about the functions is jumping out at me as being sensitive to major risk.  

 

I'd encourage you keep testing.  Maybe create another script to produce mock files around the intervals you expect files to be made in production and let it run for as long as you'd expect your monitoring/renaming script to run unattended.

 

So first part:

a) Agreed, I've tested a bit and everything looks ok, your idea of creating a seperate macro to test out this macro in real life is a good idea.

b)Yes, well that's most likely because I don't know the proper way to do it. I kinda grind my programs out then refine when I can find the cleaner version of my coding. I would appreciate any suggestion.

c) Honestly I searched around on the forums for a function to monitor new file creation and this is what came up, I tried to look at a few other functions but failed to get them working and I needed a lower resource option then just checking the directory every few seconds with the functions I know.

For the second part: 

a) You make a good point, while I think FileMove is the best option since basically I just want to rename the files, I COULD in fact copy all the files to a separate location as a backup first! Thanks for that. 

b)I just copied the example that I saw from the docs.. 

https://www.autoitscript.com/autoit3/docs/libfunctions/_WinAPI_ReadDirectoryChanges.htm

and I don't know what the _WinAPI_CloseHandle does exactly but I am guessing that's why the tray icon refuses to resond? That being said.. The program is designed to continuosly look at a domain for changes.. so wouldn't "CloseHandle" stop the function from running correctly? Am I making any sense? 

 

Anyways, you're right and I will backup all the files before renaming them and also I will test some more and make a macro to test this macro. Thanks I appreciate you taking the time to look at my code.

[spudw2k]

1 hour ago, BatMan22 said:

b)Yes, well that's most likely because I don't know the proper way to do it. I kinda grind my programs out then refine when I can find the cleaner version of my coding. I would appreciate any suggestion.

I wasn't really referring to the code language, as much as your comments and strings in the code.  That is what I meant by professionalism. 

3 hours ago, BatMan22 said:

..."Jackass, you're COPYING...Why are there MORE THEN 100 FILES run today!...THIS IS NOT POSSIBLE!...just in fucking case

I don't think there is anything wrong with putting some personality in code, it's fun in fact sometimes...but should be used appropriately; especially if there is a risk that a negative effect of your script/program will come under scrutiny of your employer and or customer.  Just ask yourself, would they find it appropriate.

Otherwise, I wish you luck...have fun and carry on...wisely.

[BatMan22]

4 minutes ago, spudw2k said:

I wasn't really referring to the code language, as much as your comments and strings in the code.  That is what I meant by professionalism. 

I don't think there is anything wrong with putting some personality in code, it's fun in fact sometimes...but should be used appropriately; especially if there is a risk that a negative effect of your script/program will come under scrutiny of your employer and or customer.  Just ask yourself, would they find it appropriate.

Otherwise, I wish you luck...have fun and carry on...wisely.

Lol.. fair enough. Honestly the program is mostly designed for my eyes only, but you're right.. just in case an error DOES happen while someone is filling in for me, it wouldn't be very appropriate. That being said it wouldn't be the end of the world, that kinda stuff wouldn't get me in trouble, only if I screwed up the files would I end up in hot water, but ill change it to something more PC.