Feedback on logic implementation and testing

Question

This is my forked branch attempting to make the code better, This is an application for having a slide show of photographs in current directory or supplied as argument.

Use keyboard controls[left to go backward and Right key to go forward and spacebar to toggle play/pause] only.

File 1. utils.py

import os
import sys

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:
        dirContent = getDirContent(_path)
        for each in dirContent:
            selFile = os.path.join(_path, each)
            if ifFilePathExists(selFile) and isExtensionSupported(selFile):
                imagesWithPath.append(selFile)
    return list(set(imagesWithPath))

def ifFilePathExists(selFile):
    return os.path.isfile(selFile)

def getDirContent(path):
    try:
        return os.listdir(path)
    except OSError:
        raise OSError("Provided path '%s' doesn't exists." % path)

File 2. slideShowBase.py

import utils

class SlideShowBase(object):
    """ SlideShowBase class contains methods that defines the 
        logic for SlideShow to plate forward or backword and 
        pause.
    """
    def __init__(self, imgLst, ppState, count, animFlag):
        self._imagesInList = imgLst
        self._pause = ppState
        self._count = count
        self.animFlag = animFlag

    def populateImagestoSlideShow(self, path):
        """ helper method to populate the list with paths 
            of images from path argument.
        """
        self._imagesInList = utils.imageFilePaths([path])

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

    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 ingestData(paths):
    """ This method is used to create a list containing
        images path to slideshow.
    """
    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"
    return imgLst

File 3. slideShow.py

import sys
import os
import utils
from PyQt4 import QtGui,QtCore
import slideShowBase


class SlideShowPics(QtGui.QMainWindow, slideShowBase.SlideShowBase):

    """ SlideShowPics class defines the methods for UI and
        working logic
    """
    def __init__(self, imgLst, num=0, flag=True, parent=None):
        super(SlideShowPics, self).__init__(parent)
        slideShowBase.SlideShowBase.__init__(self, imgLst=imgLst, ppState=False, count=num, animFlag=flag)
        self.prepairWindow()

    def prepairWindow(self):
        if not self._imagesInList:
            msgBox = QtGui.QMessageBox()
            msgBox.setText("No Image found." )
            msgBox.setStandardButtons(msgBox.Cancel | msgBox.Open);
            if msgBox.exec_() == msgBox.Open:
                self.populateImagestoSlideShow(self._browseDir())
            else:
                sys.exit()
        # 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.updateTimer = QtCore.QTimer()
        self.connect(self.updateTimer, QtCore.SIGNAL("timeout()"), self.nextImage)
        self.showFullScreen()
        self.playPause()
        #Shows the first image
        self.showImageByPath(self._imagesInList[0])

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

    def _browseDir(self):
        selectedDir = str(QtGui.QFileDialog.getExistingDirectory(None, 
                "Select Directory to SlideShow",
                os.getcwd()))
        if selectedDir:
            return selectedDir
        else:
            sys.exit()

    def nextImage(self):
        super(SlideShowPics, self).nextImage()
        self.showImageByPath(self._imagesInList[self._count])

    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 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(imgLst=None):
    app = QtGui.QApplication(sys.argv)
    window =  SlideShowPics(imgLst)
    window.show()
    window.raise_()
    sys.exit(app.exec_())

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

Test 1: test_utils.py

import unittest
import mox
import sys
import os
sys.path.insert(0, os.path.abspath( os.path.join(os.path.dirname(__file__), 
                                               '../python/') ))

import utils


class TestUtils(unittest.TestCase):
    """ docstring for Test_Utils
    """

    def setUp(self):
        self._filePaths = ["/test/file/path"]
        self.mox = mox.Mox()

    def tearDown(self):
        self.mox.UnsetStubs()
        self.mox.ResetAll()

    def test_isExtensionSupported(self):
        self.assertTrue(utils.isExtensionSupported("testFile.PNG"))
        self.assertTrue(utils.isExtensionSupported("testFile.jpg"))
        self.assertFalse(utils.isExtensionSupported("testFile.kkg"))


    def test_imageFilePaths(self):
        filePaths = self._filePaths
        fileList = ['file1.bmp']
        self.mox.StubOutWithMock(utils, 'getDirContent')
        dirContent = utils.getDirContent(filePaths[0]).AndReturn(fileList)
        self.mox.StubOutWithMock(utils,'ifFilePathExists')
        filePat = os.path.join(filePaths[0], dirContent[0])
        utils.ifFilePathExists(filePat).AndReturn(True)
        self.mox.StubOutWithMock(utils, 'isExtensionSupported')
        utils.isExtensionSupported(filePat).AndReturn(True)
        self.mox.ReplayAll()
        self.assertEquals(['/test/file/path/file1.bmp'], utils.imageFilePaths(filePaths))
        self.mox.VerifyAll()

    def test_getDirContent(self):
        self.assertRaises(OSError, utils.getDirContent, self._filePaths[0])

    def test_ifFilePathExists(self):
        self.assertFalse(utils.ifFilePathExists(self._filePaths[0]))
        self.assertTrue(utils.ifFilePathExists(__file__))


if __name__ == '__main__':
    unittest.main()

Test 2.test_slideShowbase.py

import unittest
import mox
import stubout
import sys
import os
from PyQt4 import QtGui
sys.path.insert(0, os.path.abspath( os.path.join(os.path.dirname(__file__), 
                                               '../python/') ))
from slideShowBase import SlideShowBase as _slideShowBase
import slideShowBase
import utils


class TestSlideShow(unittest.TestCase):
    """ docstring for TestSlideShow
    """
    def setUp(self):
        self.mox = mox.Mox()
        self.__stubs = stubout.StubOutForTesting()
        self.imgLst = ['/folder/test/images/test1.jpg', '/folder/test/images/test2.JPG',
        '/folder/test/images/test3.png', '/folder/test/images/test4.PNG']

    def tearDown(self):
        self.mox.UnsetStubs()
        self.mox.ResetAll()

    def test_nextImage(self):
        self.show = _slideShowBase(imgLst=self.imgLst, ppState=False, count=0, animFlag=True)
        self.show.nextImage()
        self.assertEquals(1, self.show._count)
        self.assertEquals(self.imgLst[1], self.show._imagesInList[1])

    def test_nextImage_animFlag_False(self):
        self.show = _slideShowBase(imgLst=self.imgLst, ppState=False, count=2, animFlag=False)
        self.show.nextImage()       
        self.assertEquals(1, self.show._count)
        self.assertEquals(self.imgLst[2], self.show._imagesInList[2])

    def test_ingestData_list(self):
        # monkeypatch
        self.__stubs.Set(utils, 'imageFilePaths', lambda x: self.imgLst)
        listData = slideShowBase.ingestData(self.imgLst)
        self.assertEquals(self.imgLst, listData)

    def test_ingestData_string(self):
        # monkeypatch
        self.__stubs.Set(utils, 'imageFilePaths', lambda x: self.imgLst[0])
        listData = slideShowBase.ingestData(self.imgLst[0])
        self.assertEquals(self.imgLst[0], listData)

if __name__ == '__main__':
    unittest.main()

One thing that doesnt seem to work correctly the first image on the startup is not scaled up for the first play of slideShow. Any Idea why is that ?

Also, I think I can use generator instead oflist in the slideshowbase class but cannot think of how to approach for it. Any Suggestions/hints ?

theodox · Accepted Answer · 2013-11-26 21:06:32Z

3

Two general observations:

1) Use more lookups and fewer if-else checks. For example:

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

could be replaced with the much simpler, more data-driven:

def isExtensionSupported(filename):
    """ Supported extensions viewable in SlideShow
    """
    ALLOWABLE = ("png", "jpg")
    return filename.lower()[-3:] in ALLOWABLE

2) using dual inheritance to get the functions of SlideShowBase into SlideShowPics seems more complex than needed. I'd think about just including a SlideShowBase into your SlideShowPics and letting it manage all of the image paths by delegation, while SlideShowPics handles the display. The old saying here is 'prefer composition over inheritance'

answered Nov 26, 2013 at 21:06

theodox

1,7809 silver badges9 bronze badges

\$\begingroup\$ first of, Long time no see,so u use same name on tech-artists too, anyway, as per your second observation, I had it like that what you suggested earlier but why I decided to put the logic in SlideShowBase is because I am able to test it with ease, If I have all the code in SlideShowPics then I would have to mock QtGui.QMainWindow and I would even have to mock showImageByPath method, I know it looks kinda little bit complex but now the logic is in separate class and it seems it is easily testable too \$\endgroup\$
– Ciasto piekarz
Commented Nov 27, 2013 at 5:36
1

\$\begingroup\$ I thought I recognized the name :) My only observation there would be that slideShowBase should be testable on its own - that's effectively what you're doing -- and a separate set of (simpler) tests for SlideShowPics would handle the parts of that code which are testable. GUI like that is always hard to test anyway, since so many of the bugs are not data bugs but visual errors. I always feel that easier tests = more tests = better code, so I opt for lots of dumb tests instead of finicky fancy ones -- if I have to do too much mocking it's often time for a refactor to cut down cross-coupling. \$\endgroup\$
– theodox
Commented Nov 27, 2013 at 6:23

Add a comment |

Stack Exchange Network

Feedback on logic implementation and testing

1 Answer 1

Your Answer

Hot Network Questions

Feedback on logic implementation and testing

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions