4
\$\begingroup\$

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,
                    QtCore.Qt.SmoothTransformation))

    def playPause(self):
        if not self._pause:
            self._pause = True
            self.updateTimer.start(2500)
            return self._pause
        else:
            self._pause = False
            self.updateTimer.stop()

    def keyPressEvent(self, keyevent):
        """ Capture key to exit, next image, previous image,
            on Escape , Key Right and key left respectively.
        """
        event = keyevent.key()
        if event == QtCore.Qt.Key_Escape:
            self.close()
        if event == QtCore.Qt.Key_Left:
            self.animFlag = False
            self.nextImage()
        if event == QtCore.Qt.Key_Right:
            self.animFlag = True
            self.nextImage()
        if event == 32:
            self._pause = self.playPause()

def main(paths):
    if isinstance(paths, list):
        imgLst = utils.imageFilePaths(paths)
    elif isinstance(paths, str):
        imgLst =  utils.imageFilePaths([paths])
    else:
        print " You can either enter a list of paths or single path"
    app = QtGui.QApplication(sys.argv)
    if imgLst:
        window =  SlideShowPics(imgLst)
        window.show()
        window.raise_()
        app.exec_()
    else:
        msgBox = QtGui.QMessageBox()
        msgBox.setText("No Image found in any of the paths below\n\n%s" % paths)
        msgBox.setStandardButtons(msgBox.Cancel | msgBox.Open);
        if msgBox.exec_() == msgBox.Open:
            main(str(QtGui.QFileDialog.getExistingDirectory(None, 
                "Select Directory to SlideShow",
                os.getcwd())))

if __name__ == '__main__':
    curntPaths = os.getcwd()
    if len(sys.argv) > 1:
        curntPaths = sys.argv[1:]
    main(curntPaths)

utils.py module:

import os

def isExtensionSupported(filename):
    """ Supported extensions viewable in SlideShow
    """
    if filename.endswith('PNG') or filename.endswith('png') or\
     filename.endswith('JPG') or filename.endswith('jpg'):
        return True

def imageFilePaths(paths):
    imagesWithPath = []
    for _path in paths:
        try:
            dirContent = os.listdir(_path)
        except OSError:
            raise OSError("Provided path '%s' doesn't exists." % _path)

        for each in dirContent:
            selFile = os.path.join(_path, each)
            if os.path.isfile(selFile) and isExtensionSupported(selFile):
                imagesWithPath.append(selFile)
    return list(set(imagesWithPath))
  • Pressing will move the slideshow backwards
  • Pressing will move the slideshow forward, which is also default
  • Pressing spacebar will pause
  • Pressing Esc will exit
\$\endgroup\$
1
  • \$\begingroup\$ I am thinking to pass list if valid image paths to the SlideShowPics class at the time of initialization so i do not have to have getAllImages method. any opinion? \$\endgroup\$ – Ciasto piekarz Nov 20 '13 at 17:08
2
\$\begingroup\$
  • 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.
\$\endgroup\$
1
  • \$\begingroup\$ I have updated code as per your suggestions except for the third which I guess will leave it for now. I have also moved two public methods to a separate module utils.py \$\endgroup\$ – Ciasto piekarz Nov 21 '13 at 17:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.