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

Iterator over row and columns

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

Problem

I am writing a simple raytracer just for fun. I want to iterate over rows, and each row must be an iterator over columns. A strategy I produced is the following:

class ViewPlane(object):
    def __init__(self, resolution, pixel_size, gamma):
        self.resolution = resolution
        self.pixel_size = pixel_size
        self.gamma = gamma

    def __iter__(self):

        def genR():
            class IterRow(object):
                def __init__(self, plane, row):
                    self.plane = plane
                    self.row = row

                def __iter__(self):
                    def genC():
                        for column in xrange(self.plane.resolution[1]):
                            origin = (self.plane.pixel_size * 
                                       (column - self.plane.resolution[0] / 2.0 + 0.5)
                                        , 
                                        self.plane.pixel_size *
                                         (self.row - self.plane.resolution[1] / 2.0 + 0.5)
                                        , 
                                        100.0
                                     )
                            yield ( Ray(origin = origin, direction = (0.0,0.0,-1.0)), 
                                    (self.row,column)
                                )
                        return
                    return genC()
                def __str__(self):
                    return "Row iterator on row "+str(self.row)
            for row in xrange(self.resolution[0]):
                yield IterRow(self, row)

        return genR()


Briefly stated: ViewPlane contains the pixel plane. This class implements __iter__ which defines and returns a generator function. This generator function yields instances of an internal class IterRow, which in turn has a __iter__ method that returns a generator iterating over the columns.

I personally think that it's difficult to understand, so I was wondering

Solution

class Foo:
  def __iter__(self):
     def innergen():
        yield something
     return innergen()


Is the same as:

class Foo:
  def __iter__(self):
     yield something


In other words, there is no need to define the generator, just yield the values.

Defining a class inside the method just serves to make things complicated and hard to read. Define the class outside of the method.

Having return at the end of a function doesn't do anything and makes it look like you don't know what you are doing. (I'm not saying that you don't, but whenever you include code with effect it raises a red flag. Did you think it do something?)

Unless you really need to access the data row by row I suggesting flattening the iterator.

Stylistically, I prefer to break expressions up rather then trying to split them across several lines as you've done. I think it produces less clutter.

Here is my resulting code:

class ViewPlane(object):
    def __init__(self, resolution, pixel_size, gamma):
        self.resolution = resolution
        self.pixel_size = pixel_size
        self.gamma = gamma

    def __iter__(self):
        for row in xrange(self.resolution[0]):
            for column in xrange(self.resolution[1]):
                origin_x = self.pixel_size * self.resolution[0] / 2.0 + 0.5
                origin_y = self.pixel_size * self.resolution[1] / 2.0 + 0.5
                origin = (origin_x, origin_y, 100)
                ray = Ray(origin = origin, direction = (0.0, 0.0, -1.0) )

                yield ray, (row, column)


I'm not familiar enough with RayTracers to know but I question having the iterator over ViewPlane produce Ray objects. An iterator is for producing the contents of the object. Is a really contained inside the ViewPlane? Or should the iterator produce coordinates and let the caller construct the Ray objects?

EDIT

Here is a version that doesn't flatten the iterator:

class ViewPlane(object):
    def __init__(self, resolution, pixel_size, gamma):
        self.resolution = resolution
        self.pixel_size = pixel_size
        self.gamma = gamma

    def row_iter(self, row):
        for column in xrange(self.resolution[1]):
            origin_x = self.pixel_size * self.resolution[0] / 2.0 + 0.5
            origin_y = self.pixel_size * self.resolution[1] / 2.0 + 0.5
            origin = (origin_x, origin_y, 100)
            ray = Ray(origin = origin, direction = (0.0, 0.0, -1.0) )

            yield ray, (row, column)

    def __iter__(self):
        for row in xrange(self.resolution[0]):
            yield self.row_iter(row)

Code Snippets

class Foo:
  def __iter__(self):
     def innergen():
        yield something
     return innergen()
class Foo:
  def __iter__(self):
     yield something
class ViewPlane(object):
    def __init__(self, resolution, pixel_size, gamma):
        self.resolution = resolution
        self.pixel_size = pixel_size
        self.gamma = gamma

    def __iter__(self):
        for row in xrange(self.resolution[0]):
            for column in xrange(self.resolution[1]):
                origin_x = self.pixel_size * self.resolution[0] / 2.0 + 0.5
                origin_y = self.pixel_size * self.resolution[1] / 2.0 + 0.5
                origin = (origin_x, origin_y, 100)
                ray = Ray(origin = origin, direction = (0.0, 0.0, -1.0) )

                yield ray, (row, column)
class ViewPlane(object):
    def __init__(self, resolution, pixel_size, gamma):
        self.resolution = resolution
        self.pixel_size = pixel_size
        self.gamma = gamma

    def row_iter(self, row):
        for column in xrange(self.resolution[1]):
            origin_x = self.pixel_size * self.resolution[0] / 2.0 + 0.5
            origin_y = self.pixel_size * self.resolution[1] / 2.0 + 0.5
            origin = (origin_x, origin_y, 100)
            ray = Ray(origin = origin, direction = (0.0, 0.0, -1.0) )

            yield ray, (row, column)

    def __iter__(self):
        for row in xrange(self.resolution[0]):
            yield self.row_iter(row)

Context

StackExchange Code Review Q#2424, answer score: 4

Revisions (0)

No revisions yet.