-3

So i have a 'downloads' folder where i put everything i download in my day by day work. You know we always automate everything, so I'm trying to build a simply app to run everyday to delete files older than 30 days, as i have to do this manually from time to time to avoid the folder become too big.

Here is my code :

function TForm1.deleteOldDownloads: boolean;
var
  f: string;
  i, d: Integer;
var
  sl: tstringlist;
begin
try
  FileListBox1.Directory := '\\psf\home\downloads';
  FileListBox1.refresh;
  sl := tstringlist.create;
  for i := 0 to FileListBox1.items.count - 1 do
    begin
    f := FileListBox1.Directory + '\' + FileListBox1.items[i];
    if fileexists(f) then
      d := daysbetween(FileAge(f), now)
    else
      d := 0;
    if d > 30 then // problem is here, d is always a big number, not the actually age of file
      sl.Add(f);
    end;
  if sl.count > 0 then
    begin
    for i := 0 to sl.count do
      begin
      f := sl[i];
      deletefile(f);
      end;
    end;
  sl.Free;
except
  on e: Exception do
    begin     
    end;
end;

Problem is "d" variable is returning very big numbers like 1397401677, even if the file has only 1 day.

The only detail here is i run Windows in a Parallels virtual machine and the "\psf\home\downloads" folder is on Mac, but i can access this folderl normally using Windows Explorer, so for Delphi is like a regular local folder.

What am i missing ?

delphirules
  • 6,443
  • 17
  • 59
  • 108
  • 2
    I don't see why a value returned by that FileAge overload could be compared against Now. What that FileAge overload returns isn't a date time so far as I know. I would expect the documentation would tell you what FileAge returns. Yeah, just checked, its all there. You've spent time writing the question, but you'd have got the solution by reading the docs quicker than the time you spent writing this. After all the years on this site I am still amazed that people don't realise that documentation exists and can be read. – David Heffernan Oct 12 '21 at 13:16
  • 2
    @DavidHeffernan is very right. In this case you cannot even *guess* what `FileAge` returns. I mean, it could be the age in tenths of a millisecond, in which case your value would be about a day! – Andreas Rejbrand Oct 12 '21 at 13:23
  • 2
    Also: (1) You *must* protect your resources using `try..finally` blocks. (2) `sl[sl.Count]` is wrong. (3) `if sl.count > 0 then` is not necessary; `for i := 0 to -1` is perfectly valid code and yields no iterations. (4) Don't silently ignore exceptions. – Andreas Rejbrand Oct 12 '21 at 13:30
  • You don't need the SL. You can delete directly. Furthermore, you don't need the 'refresh'. – Gabriel Oct 15 '21 at 07:50
  • 1
    Don't silently ignore exceptions. – Gabriel Oct 15 '21 at 07:51
  • @Z80 What the eye doesn't see the heart doesn't grieve over :-p – delphirules Oct 19 '21 at 12:34
  • 1
    or at least move the try/except around "deletefile"... – Gabriel Oct 20 '21 at 07:50

3 Answers3

7

Did you read the documentation for FileAge? The first day in programming school, you are taught "When you start using a new function or API, you begin by reading its documentation." In this case, the function's documentation says

The [one-argument] overloaded version of FileAge is deprecated.

So you are using a function you shouldn't be using.

Still, this function should probably still work.

But what do you expect it to return? Well, obviously the thing that the docs say it should return:

The first overload returns an integer that represents the OS time stamp of the file. The result can be later converted to a TDateTime using the FileDateToDateTime function.

But when you use this in DaysBetween, you assume it already is a TDateTime!

Why is FileAge returning unexpected values?

It isn't. It is probably returning exactly the thing its documentation says it should return.

Andreas Rejbrand
  • 105,602
  • 8
  • 282
  • 384
  • All the more reason to use the 2-param `FileAge()` instead, since its output value is a `TDateTime` not an `Integer`. – Remy Lebeau Oct 12 '21 at 20:20
4

You are using the older version of FileAge() that returns a timestamp in DOS numeric format, but you are treating it as a TDateTime, which it is not. As the FileAge documentation says:

The first overload returns an integer that represents the OS time stamp of the file. The result can be later converted to a TDateTime using the FileDateToDateTime() function.

So, do what the documentation says to do, eg:

var
  age: Integer;

age := FileAge(f);
if age <> -1 then
  d := DaysBetween(FileDateToDateTime(age), Now)

Otherwise, use the newer version of FileAge() that outputs a TDateTime to begin with, eg:

var
  dt: TDateTime;

if FileAge(f, dt) then
  d := DaysBetween(dt, Now)
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
1

This is NOT a direct answer to your question, but I cannot post it as a comment. So, the thing is, you should never delete user files directly. What if you make a mistake? What if the user of your program makes a mistake? Always delete files to Recycle Bin:

{--------------------------------------------------------------------------------------------------
   DELETE FILE
   Deletes a file/folder to RecycleBin.
   Old name: Trashafile
   Note related to UNC: The function won't move a file to the RecycleBin if the file is UNC. MAYBE it was moved to the remote's computer RecycleBin
--------------------------------------------------------------------------------------------------}
function RecycleItem(CONST ItemName: string; CONST DeleteToRecycle: Boolean= TRUE; CONST ShowConfirm: Boolean= TRUE; CONST TotalSilence: Boolean= FALSE): Boolean;
VAR
   SHFileOpStruct: TSHFileOpStruct;
begin
 FillChar(SHFileOpStruct, SizeOf(SHFileOpStruct), #0);
 SHFileOpStruct.wnd              := Application.MainForm.Handle;                                   { Others are using 0. But Application.MainForm.Handle is better because otherwise, the 'Are you sure you want to delete' will be hidden under program's window }
 SHFileOpStruct.wFunc            := FO_DELETE;
 SHFileOpStruct.pFrom            := PChar(ItemName+ #0);                                           { ATENTION!   This last #0 is MANDATORY. See this for details: http://stackoverflow.com/questions/6332259/i-cannot-delete-files-to-recycle-bin  -   Although this member is declared as a single null-terminated string, it is actually a buffer that can hold multiple null-delimited file names. Each file name is terminated by a single NULL character. The last file name is terminated with a double NULL character ("\0\0") to indicate the end of the buffer }
 SHFileOpStruct.pTo              := NIL;
 SHFileOpStruct.hNameMappings    := NIL;

 if DeleteToRecycle
 then SHFileOpStruct.fFlags:= SHFileOpStruct.fFlags OR FOF_ALLOWUNDO;

 if TotalSilence
 then SHFileOpStruct.fFlags:= SHFileOpStruct.fFlags OR FOF_NO_UI
 else
   if NOT ShowConfirm
   then SHFileOpStruct.fFlags:= SHFileOpStruct.fFlags OR FOF_NOCONFIRMATION;

 Result:= SHFileOperation(SHFileOpStruct)= 0;

 //DEBUG ONLY if Result<> 0 then Mesaj('last error: ' + IntToStr(Result)+ CRLF+ 'last error message: '+ SysErrorMessage(Result));
 //if fos.fAnyOperationsAborted = True then Result:= -1;
end;

Also, you don't need that obsolete control to get the files in a folder. You can use this:

{ FIND FILES }
function ListFilesOf(CONST aFolder, FileType: string; CONST ReturnFullPath, DigSubdirectories: Boolean): TStringList;
{ If DigSubdirectories is false, it will return only the top level files,
  else it will return also the files in subdirectories of subdirectories.
  If FullPath is true the returned files will have full path.
  FileType can be something like '*.*' or '*.exe;*.bin'
  Will show also the Hidden/System files.
  Source Marco Cantu Delphi 2010 HandBook

   // Works with UNC paths}
VAR
  i: Integer;
  s: string;
  SubFolders, filesList: TStringDynArray;
  MaskArray: TStringDynArray;
  Predicate: TDirectory.TFilterPredicate;

   procedure ListFiles(CONST aFolder: string);
   VAR strFile: string;
   begin
    Predicate:=
          function(const Path: string; const SearchRec: TSearchRec): Boolean
          VAR Mask: string;
          begin
            for Mask in MaskArray DO
              if System.Masks.MatchesMask(SearchRec.Name, Mask)
              then EXIT(TRUE);
            EXIT(FALSE);
          end;

    // Long paths will raise an EPathTooLongexception exception, so we simply don't process those folders
    if Length(aFolder) > MAXPATH
    then exit;

    filesList:= TDirectory.GetFiles (aFolder, Predicate);
    for strFile in filesList DO
     if strFile<> ''                                                                                 { Bug somewhere here: it returns two empty entries ('') here. Maybe the root folder?  }
     then Result.Add(strFile);
   end;

begin
 { I need this in order to prevent the EPathTooLongexception (reported by some users) }
 if aFolder.Length >= MAXPATH then
  begin
   MesajError('Path is longer than '+ IntToStr(MAXPATH)+ ' characters!');
   EXIT(NIL);
  end;

 if NOT System.IOUtils.TDirectory.Exists (aFolder)
 then RAISE exception.Create('Folder does not exist! '+ CRLF+ aFolder);

 Result:= TStringList.Create;

 { Split FileType in subcomponents }
 MaskArray:= System.StrUtils.SplitString(FileType, ';');

 { Search the parent folder }
 ListFiles(aFolder);

 { Search in all subfolders }
 if DigSubdirectories then
  begin
   SubFolders:= TDirectory.GetDirectories(aFolder, TSearchOption.soAllDirectories, NIL);
   for s in SubFolders DO
    begin
     if ccIO.DirectoryExists(s)                                                                     { This solves the problem caused by broken 'Symbolic Link' folders }
     then ListFiles(s);
    end;
  end;

 { Remove full path }
 if NOT ReturnFullPath then
  for i:= 0 to Result.Count-1 DO
   Result[i]:= TPath.GetFileName(Result[i]);
end;

The code above is from: https://github.com/GodModeUser/Delphi-LightSaber

Gabriel
  • 20,797
  • 27
  • 159
  • 293