• 14
name

A PHP Error was encountered

Severity: Notice

Message: Undefined index: userid

Filename: views/question.php

Line Number: 191

Backtrace:

File: /home/prodcxja/public_html/questions/application/views/question.php
Line: 191
Function: _error_handler

File: /home/prodcxja/public_html/questions/application/controllers/Questions.php
Line: 433
Function: view

File: /home/prodcxja/public_html/questions/index.php
Line: 315
Function: require_once

I'm combining existing import and export functions in order to reduce the number of times the user gets told the connection was refused in the event that the connection does get refused. The library I'm calling has separate import and export functions, as well as a combined import/export function. The export function requires the list of files to be exported, while the combined function figures out the list by itself (and internally calls the export function with this list). Since either import or export can be turned off by the user, I want to only get the list of files if necessary.

I have come up with this code:

List<File> files;
if (mExport)
    files = ListFiles();

if (mExport && mImport && files.size() > 0) // Error is on files
    DoExportAndImport();
else if (mImport)
    DoImport();
else if (mExport && files.size() > 0) // No error here
    DoExport(files);

The second if statement gets marked with the error Variable 'files' might not have been initialized (but not the last one).

Help me with my reasoning here: if mExport is true, then files gets initialized; but if mExport is false, then the second if statement gets short-circuited and never reaches files, so it doesn't matter that files hasn't been initialized, because it isn't being used.

Am I overlooking something here, or is this just too complex a situation for the compiler to work out? If the latter, is there any way to tell the compiler to knock it off, I've got it handled?

For the record, initializing files = new ArrayList<>() in the declaration does silence the compiler, as does files = null, though that obviously leads to other errors; but it just feels like such a waste to initialize a value that I know is never going to be used or referred to in any way.

      • 2
    • files = Collections.emptyList() might be a better "dummy" initializer, as this does no allocation.

Your logic is sound in that files will always be initialized with each case in which files is referenced. However, as you've suspected, the compiler doesn't go that far. Its static analysis does not take into account the values of variables to determine whether a certain condition will always be true or false.

In this case, the compiler doesn't take into account that files is initialized if mExport is true. It only sees the possibility that files is not initialized from the top if statement, and that it is reference below that statement.

You can help by rearranging your logic so that it's only referenced from within the if statement block.

List<File> files;
if (mExport) {
    files = ListFiles();
    if (mImport && files.size() > 0) {
        DoExportAndImport();
    } else if (files.size() > 0) {
        DoExport(files);
    }
} else if (mImport) {
    DoImport();
}
  • 1
Reply Report
      • 1
    • This almost gets me there, but it doesn't call DoImport() if there aren't any files. It will require an extra if case to handle that.
      • 2
    • But it did give me the inspiration to set mExport to the result of files.size() > 0 in the first if statement, which I think will work.

Static analyzer is not ultimate AI. Sometimes it may give false positives. You should be able to mute complaints on this particular line, however I would suggest to rather rework the logic of the whole code block to make it simpler.

Also files = null is redundant as files is null unless initialized otherwise. It is just fooling analyser.

  • 0
Reply Report