Odd behavior when populating a Table

[Kyle_Katarn]

Hi there, I guess this marks my first post here. I've been going at my own pace with Qt for a few weeks (and I have to say it's got to be my favorite Application development library of all time!)

Anyway, since I'm a big music person, I have a good lot of files with different naming conventions and such, so I thought it would be cool to apply my skills as a programmer and write a simple batch music file renamer as my first independent Qt app. I have made some surprising progress in the first day, I have been able to specify a folder, and populate a table with the files in that folder (even being able to filter out to specific filetypes!) I know it must sound boring to you guys, but as a programmer who's spent most of his time with more realtime application development (games and such), I think it's just magical.

I know my title states "Odd behavior," but in actual fact it's probably something so simple that I've overlooked and need a good bonk over the head for, but anyway, before I populate my table, all I have are three columns "File Name" (I'm still looking for a way to make this column non-editable), "Song Title" and "Song Number." The idea is that you're supposed load a folder (this is assuming you dont have all your mp3's in one folder, but rather separate them into separate folders by artist, album, whatever like myself), and then set some Global fields (artist name, album title, etc), then you can go into the local fields and set them yourself (or I could try to develop an algorithm to do it for you if I ever are smart to do that enough), then hit the rename button and its all done for you.

Before I load in a folder, my application looks like this:



And after I load in say Aerosmith's Ultimate Hits CD1:



As you can see, the Column names have been replaced by numbers. Now I think the solution to this is probably very simple (problems like these usually are), but I cant for the life of me think of what is going on...

Also the obligatory source code:

void BatchRenamer::populateTable( QString folderName )
{
	// no need to validate folderName because it's already done in openFolder()
	QDir activeDirectory( folderName );
	QStringList fileList = activeDirectory.entryList( QStringList() << "*.mp3" << "*.wma", QDir::Files );
 
	// clear the current file list before doing anything
	ui.tblFiles->clear();
	if ( !oldFileList.isEmpty() )
	{
		for ( int i = 0 ; i < oldFileList.size() ; i++ )
		{
			ui.tblFiles->removeRow(i);
			ui.tblFiles->hideRow(i);
		}
	}
 
 
	for ( int i = 0; i < fileList.size() ; i++ )
	{
		QTableWidgetItem* item = new QTableWidgetItem( fileList.at(i));
		ui.tblFiles->insertRow(i);
		ui.tblFiles->setItem(i, 0, item);
		ui.tblFiles->showRow(i);
	}
 
	oldFileList = fileList;
}

void BatchRenamer::populateTable( QString folderName )
{
	// no need to validate folderName because it's already done in openFolder()
	QDir activeDirectory( folderName );
	QStringList fileList = activeDirectory.entryList( QStringList() << "*.mp3" << "*.wma", QDir::Files );
	
	// clear the current file list before doing anything
	ui.tblFiles->clear();
	if ( !oldFileList.isEmpty() )
	{
		for ( int i = 0 ; i < oldFileList.size() ; i++ )
		{
			ui.tblFiles->removeRow(i);
			ui.tblFiles->hideRow(i);
		}
	}
	

	for ( int i = 0; i < fileList.size() ; i++ )
	{
		QTableWidgetItem* item = new QTableWidgetItem( fileList.at(i));
		ui.tblFiles->insertRow(i);
		ui.tblFiles->setItem(i, 0, item);
		ui.tblFiles->showRow(i);
	}

	oldFileList = fileList;
}

To copy to clipboard, switch view to plain text mode 

And the method that calls populateTable(), which itself is called by a connection of the Find folder button clicked signal:

void BatchRenamer::openFolder()
{
	QString folderName = QFileDialog::getExistingDirectory( this, "Select Media Folder" );
 
	if( !folderName.isEmpty() )
	{
		ui.ledSelectedFolder->setText(folderName);
		populateTable( folderName );
	}
}

void BatchRenamer::openFolder()
{
	QString folderName = QFileDialog::getExistingDirectory( this, "Select Media Folder" );

	if( !folderName.isEmpty() )
	{
		ui.ledSelectedFolder->setText(folderName);
		populateTable( folderName );
	}
}

To copy to clipboard, switch view to plain text mode 

Thank you for your time. I would really appreciate instead of someone posting the code saying this works: go, take the time and explain to me why it's doing what it's doing and how I can go about fixing it. Of course, you dont have to though.

tl;dr:
When I populate my table, the column names get replaced with numbers.

[JohannesMunk]

Hi Kyle!

Welcome! That's a very nice first post! You put a lot of effort into it!

But as you suspected the solution to your problem is very simple: There is a difference between clear() and clearContents() ! The former clears all header-cells too! The later doesn't!

To make the fileName items readonly you will have to remove the Qt::ItemIsEditable. That should work like this:

QTableWidgetItem* item = new QTableWidgetItem( fileList.at(i));
item->setFlags(item->flags() & ~Qt::ItemIsEditable);

QTableWidgetItem* item = new QTableWidgetItem( fileList.at(i));
item->setFlags(item->flags() & ~Qt::ItemIsEditable);

To copy to clipboard, switch view to plain text mode 

I also would like to point out, that it is much faster to set the rowCount at once instead of removing and adding each row one by one:

ui.tblFiles->setRowCount(fileList.size());

ui.tblFiles->setRowCount(fileList.size());

To copy to clipboard, switch view to plain text mode 

HIH

Johannes

[Kyle_Katarn]

Haha, well that's the thing about first impressions, you only get to make one (well, with a single account anyway). And D'oh! I cant believe I missed the clearContents() function! I stared at the QTableWidget documentation for at least half an hour! Must have had something to do with it being nearly 3am when I posted the topic... Anyway, thanks a bunch for your help, it all worked exactly as I wanted it to!

Thanks again,
Kyle

[Kyle_Katarn]

I seem to have hit another snag, this time to do with the connections of the tables Cells. im trying to set up a connection with the hopes of each time a cell is clicked on (or navigated to via the arrow keys), so I'm guessing a cellPressed(int, int) would be suitable, although Qt doesn't seem to think so...

When I run the program, the list populates properly, but when I click on a cell, regardless on if it's empty or not (which when it first loads up, it is empty), it will throw an unhandled exception:

Unhandled exception at 0x657d748c (QtGuid4.dll) in BatchRenamer.exe: 0xC0000005: Access violation reading location 0x00000000.

Then it will throw me out at line 108 in qtablewidget.h

inline QString text() const
{ return data(Qt::DisplayRole).toString(); }

inline QString text() const
{ return data(Qt::DisplayRole).toString(); }

To copy to clipboard, switch view to plain text mode 

Now if I had to guess, it would be due to the fact that there's nothing in the cell, probably not even an empty string, but I'm not too sure how I can handle and catch this exception.

Here's the code that sets up the connection:

void BatchRenamer::populateTable( QString folderName )
{
 
	// no need to validate folderName because it's already done in openFolder()
	QDir activeDirectory( folderName );
	QStringList fileList = activeDirectory.entryList( QStringList() << "*.mp3" << "*.wma", QDir::Files );
 
	// clear the current file list before doing anything
	ui.tblFiles->clearContents();
	disconnect( ui.tblFiles );
	if ( !oldFileList.isEmpty() )
	{
		for ( int i = 0 ; i < oldFileList.size() ; i++ )
		{
			ui.tblFiles->removeRow(i);
			ui.tblFiles->hideRow(i);
		}
	}
 
	if ( !fileList.isEmpty() )
	{
		for ( int i = 0; i < fileList.size() ; i++ )
		{
			QTableWidgetItem* item = new QTableWidgetItem( fileList.at(i) );
			// make the items in the first column read only
			item->setFlags(item->flags() & ~Qt::ItemIsEditable);
 
			ui.tblFiles->setRowCount( fileList.count() );
			ui.tblFiles->setItem( i, 0, item );
			ui.tblFiles->showRow( i );
 
		}
	}
	ui.tblFiles->resizeColumnToContents( 0 );
	oldFileList = fileList;
 
	// hook up the newly made table entries
	connect( ui.tblFiles, SIGNAL( cellChanged(int, int)), this, SLOT( changeLocalSongData( int, int )));
	connect( ui.tblFiles, SIGNAL( cellPressed(int, int)), this, SLOT( changeLocalSongData( int, int )));
}

void BatchRenamer::populateTable( QString folderName )
{

	// no need to validate folderName because it's already done in openFolder()
	QDir activeDirectory( folderName );
	QStringList fileList = activeDirectory.entryList( QStringList() << "*.mp3" << "*.wma", QDir::Files );
	
	// clear the current file list before doing anything
	ui.tblFiles->clearContents();
	disconnect( ui.tblFiles );
	if ( !oldFileList.isEmpty() )
	{
		for ( int i = 0 ; i < oldFileList.size() ; i++ )
		{
			ui.tblFiles->removeRow(i);
			ui.tblFiles->hideRow(i);
		}
	}
	
	if ( !fileList.isEmpty() )
	{
		for ( int i = 0; i < fileList.size() ; i++ )
		{
			QTableWidgetItem* item = new QTableWidgetItem( fileList.at(i) );
			// make the items in the first column read only
			item->setFlags(item->flags() & ~Qt::ItemIsEditable);

			ui.tblFiles->setRowCount( fileList.count() );
			ui.tblFiles->setItem( i, 0, item );
			ui.tblFiles->showRow( i );
			
		}
	}
	ui.tblFiles->resizeColumnToContents( 0 );
	oldFileList = fileList;

	// hook up the newly made table entries
	connect( ui.tblFiles, SIGNAL( cellChanged(int, int)), this, SLOT( changeLocalSongData( int, int )));
	connect( ui.tblFiles, SIGNAL( cellPressed(int, int)), this, SLOT( changeLocalSongData( int, int )));
}

To copy to clipboard, switch view to plain text mode 

Here's the changeLocalSongData(int,int) code

void BatchRenamer::changeLocalSongData( int row, int column )
{
	if( column == 1)
	{
		songName = ui.tblFiles->item(row, column)->text();
		updatePreview();
	}
}

void BatchRenamer::changeLocalSongData( int row, int column )
{
	if( column == 1)
	{
		songName = ui.tblFiles->item(row, column)->text();
		updatePreview();
	}
}

To copy to clipboard, switch view to plain text mode 

And it may be pointless to do so, but here's the updatePreview() function (all this is really to display some hidden information down the bottom of the window (under the "Preview:" label in the screenshot above).

void BatchRenamer::updatePreview()
{
	QString previewText = "\t"; 
 
	if ( artistName != "")
	{
			previewText += artistName;
	}
	if (albumTitle != "")
	{
		previewText += spacer + albumTitle;
	}
	if (songName != "")
	{
		previewText += spacer + songName;
	}
 
	ui.lblPreviewFileName->setText( previewText );
}

void BatchRenamer::updatePreview()
{
	QString previewText = "\t"; 

	if ( artistName != "")
	{
			previewText += artistName;
	}
	if (albumTitle != "")
	{
		previewText += spacer + albumTitle;
	}
	if (songName != "")
	{
		previewText += spacer + songName;
	}

	ui.lblPreviewFileName->setText( previewText );
}

To copy to clipboard, switch view to plain text mode 

I've already tried catching the exception and handling it in the changeLocalSongData() function via this code:

if (ui.tblFiles->item(row,column)->text() != "")
		{
			songName = ui.tblFiles->item(row, column)->text();
			updatePreview();
		}
		else
			ui.tblFiles->item(row,column)->setText("");

if (ui.tblFiles->item(row,column)->text() != "")
		{
			songName = ui.tblFiles->item(row, column)->text();
			updatePreview();
		}
		else
			ui.tblFiles->item(row,column)->setText("");

To copy to clipboard, switch view to plain text mode 

I have no doubt that this is a PEBKAC (problem existing between keyboard and char) error.

I would appreciate any help.

Thanks in advance.

P.S. If I comment out the connection to cellPressed(), the cellChanged connection will work without a hitch.

[JohannesMunk]

Hi Kyle!

First of all, you did not quite get the part with the setRowCount. You don't need to set it in each iteration of your loop, but only once outside:

void BatchRenamer::populateTable( QString folderName ) 
{
     // no need to validate folderName because it's already done in openFolder()
     QDir activeDirectory( folderName );
     QStringList fileList = activeDirectory.entryList( QStringList() << "*.mp3" << "*.wma", QDir::Files );
     // clear the current file list before doing anything
     ui.tblFiles->clearContents();
 
     ui.tblFiles->setRowCount( fileList.count() );
      // if ( !fileList.isEmpty() )
      // check not necessary because the for loop will not start, when fileList.size() is 0.
 
     for ( int i = 0; i < fileList.size() ; i++ )
     {
         QTableWidgetItem* item = new QTableWidgetItem( fileList.at(i) );
         // make the items in the first column read only
         item->setFlags(item->flags() & ~Qt::ItemIsEditable);
         ui.tblFiles->setItem( i, 0, item );
     }
     ui.tblFiles->resizeColumnToContents( 0 );
     oldFileList = fileList; }

void BatchRenamer::populateTable( QString folderName ) 
{
     // no need to validate folderName because it's already done in openFolder()
     QDir activeDirectory( folderName );
     QStringList fileList = activeDirectory.entryList( QStringList() << "*.mp3" << "*.wma", QDir::Files );
     // clear the current file list before doing anything
     ui.tblFiles->clearContents();

     ui.tblFiles->setRowCount( fileList.count() );
      // if ( !fileList.isEmpty() )
      // check not necessary because the for loop will not start, when fileList.size() is 0.

     for ( int i = 0; i < fileList.size() ; i++ )
     {
         QTableWidgetItem* item = new QTableWidgetItem( fileList.at(i) );
         // make the items in the first column read only
         item->setFlags(item->flags() & ~Qt::ItemIsEditable);
         ui.tblFiles->setItem( i, 0, item );
     }
     ui.tblFiles->resizeColumnToContents( 0 );
     oldFileList = fileList; }

To copy to clipboard, switch view to plain text mode 

BTW: QList::count(), QList::size(), QList::length() are all the same, but you should probably decide which one you prefer and then use that one consistently.

Now for your connection problem!

Do not setup the connection each time your list is updated but rather in the constructor of your mainwindow. The connection is stored between the tblFiles object and your this-object. Not for individual items.

BatchRenamer::BatchRenamer()
{
    ....
    // hook up the table
    connect( ui.tblFiles, SIGNAL( cellChanged(int, int)), this, SLOT( changeLocalSongData( int, int ))); 
    connect( ui.tblFiles, SIGNAL( cellPressed(int, int)), this, SLOT( changeLocalSongData( int, int )));  
    ...
}

BatchRenamer::BatchRenamer()
{
    ....
    // hook up the table
    connect( ui.tblFiles, SIGNAL( cellChanged(int, int)), this, SLOT( changeLocalSongData( int, int ))); 
    connect( ui.tblFiles, SIGNAL( cellPressed(int, int)), this, SLOT( changeLocalSongData( int, int )));  
    ...
}

To copy to clipboard, switch view to plain text mode 

The access violation happens, because the row and col parameter point to a non existing item. thus ui.tblFiles->item(row,col) returns 0. If you want to check against that, don't check the text for beeing empty but for item returning 0. Your check throws an access violation, because in accessing text() you are trying to derefer a pointer to an object, that points to 0 instead to a valid object.

And all that happens, because you did not create items for the other columns I guess.

Johannes

[Kyle_Katarn]

Dude, you're awesome! Thanks again for the very informative post. I've managed to get everything once again working the way I want it to, thanks to you. =P

Thanks again!

[JohannesMunk]

You are welcome!

Joh

[Kyle_Katarn]

Sorry if this is considered a bump, but I want to say that my program is currently working exactly as I want it to (that is, it actually renames stuff!)

Thanks for all the help.

[JohannesMunk]

:-> That's good news!

I think a bump is only when you are desparately fishing for an answer..

Best of luck for your next projects,

Joh