HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Simple PyQt4 image slide show player

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
showpyqt4simpleimageslideplayer

Problem

I have written a slideshow application and I am looking for some reviews:

```
import sys
import os
import utils

from PyQt4 import QtGui,QtCore

class SlideShowPics(QtGui.QMainWindow):

""" SlideShowPics class defines the methods for UI and
working logic
"""
def __init__(self, imgLst, parent=None):
super(SlideShowPics, self).__init__(parent)
# self._path = path
self._imageCache = []
self._imagesInList = imgLst
self._pause = False
self._count = 0
self.animFlag = True
self.updateTimer = QtCore.QTimer()
self.connect(self.updateTimer, QtCore.SIGNAL("timeout()"), self.nextImage)
self.prepairWindow()
self.nextImage()

def prepairWindow(self):
# Centre UI
screen = QtGui.QDesktopWidget().screenGeometry(self)
size = self.geometry()
self.move((screen.width()-size.width())/2, (screen.height()-size.height())/2)
self.setStyleSheet("QWidget{background-color: #000000;}")
self.setWindowFlags(QtCore.Qt.WindowStaysOnTopHint)
self.buildUi()
self.showFullScreen()
self.playPause()

def buildUi(self):
self.label = QtGui.QLabel()
self.label.setAlignment(QtCore.Qt.AlignCenter)
self.setCentralWidget(self.label)

def nextImage(self):
""" switch to next image or previous image
"""
if self._imagesInList:
if self._count == len(self._imagesInList):
self._count = 0

self.showImageByPath(
self._imagesInList[self._count])

if self.animFlag:
self._count += 1
else:
self._count -= 1

def showImageByPath(self, path):
if path:
image = QtGui.QImage(path)
pp = QtGui.QPixmap.fromImage(image)
self.label.setPixmap(pp.scaled(
self.label.size(),
QtCore.Qt.KeepAspectRatio,

Solution


  • When there are no images to show, you print a message to console and exit without showing the GUI. The problem is that a GUI user probably won't see the console and is left wondering why the program won't start. You could show your window with the text in a label, instead.



  • The list of images gets loaded in an indirect way when nextImage sees the list is empty. It would be clearer to pass the list in explicitly, like you already propose in your recent comment.



  • You filter filenames for png and jpg endings in two places. If you would introduce another format, it would be easy forget to update both functions. Also, I think it would be better to let Qt decide what it is able to display. So, try to show every file and skip quietly to the next one in case of error.



  • _openFolder is not used. Please delete dead code before submitting for review.

Context

StackExchange Code Review Q#35506, answer score: 2

Revisions (0)

No revisions yet.