Method/ Choice of design /Robustness question

ODБOï

Hi,

Please give me your feedback on my method. I have to save images continuously and start deleting oldest images when total number exceeds a maximum value, FIFO.
I'm dealing with the filenames to do save / delete, i wonder if there is a more 'natural' way to do this.
saveToImage() is called maybe 9 ~ 13 times per second.

   void saveToImage(QImage i){
        QString name = "Hmi_frame_";
        name.append(QString::number(inc)); // inc is the number of images, incremented each time
        name.append(".png");

//not important constructing a dir with date of day       
 QString dirName(WORKINGDIR);
        dirName.append(QDate::currentDate().toString());
        dirName.append("/");
        QDir d(dirName);
        if(!d.exists(dirName)){
            d.mkpath(".");
            qDebug()<<"making dir..";
        }
//--

        i.save(d.absolutePath()+ "/" + name,"PNG");
        if(inc>=FRAMECPT){ //  FRAMECPT is the maximum i want : 40000
            QObject::disconnect(this,&QVNCClientWidget::saveImg,this,&QVNCClientWidget::saveToImage);
            deleteOldest();
        }
        inc++;
        emit incUpdated(inc);
    }

    void deleteOldest(){
        QString path = WORKINGDIR;
        path.append("/Hmi_frame_");
        path.append(QString::number(inc-FRAMECPT));
        path.append(".png");

        QFile file(path);

        if(!file.exists()){
            qDebug()<<"FILE NOT FOUND ERROR !";
            return; 
        }

        file.remove();
        QObject::connect(this,&QVNCClientWidget::saveImg,this,&QVNCClientWidget::saveToImage);
    }

JonB

@LeLev
Don't know what you mean by "i wonder if there is a more 'natural' way to do this". If you want to retain 40,000, and want to be in control of the filenames, then what you have does that.

Maybe a touch of tidying. If for whatever reason the file to delete is not found, your code does not reconnect your slot. (Not sure why you need to disconnect/connect slot each time you delete, but that's up to you.)

ODБOï

hi
@JonB said in Method/ Choice of design /Robustness question:

If you want to retain 40,000, and want to be in control of the filenames

I don't necessarily need to be in control of filenames, all i need is keep 40000 last files, that's why i ask if someone knows a better way to do this.

@JonB said in Method/ Choice of design /Robustness question:

Maybe a touch of tidying. If for whatever reason the file to delete is not found, your code does not reconnect your slot

Exact, i saw that when pasted my code here. I will correct this Thanks.

I disconnect/connect every time for safety, saveToImage() will be called only after the file is saved. Is this usless ?

JonB

@LeLev
Well I only see two ways of maintaining 40,000 files:

Use your way, which in principle works.
Allow Qt/C++/whatever to create temporary filenames. But then you're going to have to maintain your own list/array of all the files created so that you can go back and delete the 40,000'th oldest. That's too many --- I'd rather maintain the counter myself.

I suspect the disconnect/re-connect is unnecessary, in that I don't see anything in deleteOldest() that would care when it is called, even if it was re-entered. I also don't see how saveToImage() would be re-entered while it is running/deleting anyway, do you? But I may be wrong and I guess it does no harm if we're not sure.

ODБOï

@JonB said in Method/ Choice of design /Robustness question:

Allow Qt/C++/whatever to create temporary filenames. But then you're going to have to maintain your own list/array of all the files created so that you can go back and delete the 40,000'th oldest

i absolutely don't know how this works, i will check

@JonB said in Method/ Choice of design /Robustness question:

I also don't see how saveToImage() would be re-entered while it is running/deleting anyway, do you?

No, is is always called from the same place

JonB

@LeLev

i absolutely don't know how this works, i will check

What is there you don't know? There should be nothing to check? I am saying/agreeing that allowing a system call to create the temporary file names is not a good idea for your situation, so what do you need to investigate?

ODБOï

@JonB you said you see two ways right ?
first is ~like me
and se second is : Allow Qt/C++/whatever to create temporary filenames. But then you're going to have to maintain your own list/array of all the files created so that you can go back and delete the 40,000'th oldest. That's too many --- I'd rather maintain the counter myself.

I don't get this, i thought this was another approch.
what do you mean by 'allowing a system call to create the temporary file names' ?

sorry

JonB

@LeLev
There will be C/C++/Qt functions to create temporary files with temporary names for you, e.g. http://doc.qt.io/qt-5/qtemporaryfile.html. That's what we'd normally use to create temp files, and it's very nice. But I'm saying: it won't "naturally" do anything about your 40,000 limit. So it is another way of creating temp files than your code, but I'm saying I don't think it fits your situation at all as you'd have to manage storing up the names in order to do the deletes yourself. Consequently, although it is another approach, I would not use such a function but rather stick with the way you have now.

One other observation: although you generate a directory name from today's date, up to 40,000 files will be stored in that directory on a given day. Now, NTFS may have improved on this, but in the olden days/my experience Windows is not great at working with directories containing anywhere near that number of files (e.g. try browsing into the directory with File Explorer), it might get slow creating/deleting that many files in one directory. You could split your files over multiple directories if you do find it's too many....

ODБOï

@JonB thank you very much for advices !