0

Consider this piece of code:

class Page(object):
  def __init__(self, name, title):
    self.name   = name
    self.title  = title
    self.selected = False
  def select(self):                 <-- How can I make this method work?
    for Page in Pages:
        Page.selected = False
    self.selected = True
class Website(object):
  def __init__(self):
    self.index    = Page("index", "Home")
    self.settings = Page("settings", "Settings")
    self.users    = Page("users", "Users")
    self.logs     = Page("logs", "Logs")
    self.faq      = Page("faq", "FAQ")
  def __iter__(self):
    return iter([self.index, self.settings, self.users, self.logs, self.faq])
Pages = Website()

What I am trying to do seems sort of illegal. Nevertheless, I am sure there is a way to do it. It seems like I might have to rewrite get method somewhere. Thank you very much for your help!

Here is the way I was intending to use those classes using Bottlepy:

Setting Pages:

@route('/')
@route('/<selectedPage>')
@route('/<selectedPage>/')
def dynamic_routing(selectedPage='index'):
  for Page in Pages:
     if selectedPage == Page.name:
            Page.select()
  return template('default')

Retrieving Page info (inside Bottlepy template):

%for Page in Pages:
    %if Page.selected:
        <title>{{Page.title}}</title>                
    %else:
        <title>Page Not Found</title>
    %end
%end

I edited code to a working version now. Thanks everyone for such a fast input!!! You guys rock! Still probably not the best approach but I can not think of another way to solve it at the moment.

BalusC
  • 1,082,665
  • 372
  • 3,610
  • 3,555
Barmaley
  • 1,232
  • 20
  • 27
  • 1
    [PEP-8](http://www.python.org/dev/peps/pep-0008/#class-names) recommended only using `CapWords` for class names - it makes code much more readable. Likewise, it recommends against lining up assignments. There are also no outer or inner classes here (although such concepts are pretty irrelevant in Python anyway). – Gareth Latty Mar 05 '13 at 22:41
  • It'll work already, just add a `self` parameter. – Martijn Pieters Mar 05 '13 at 22:42
  • @MartijnPieters It won't, you need an instance of `Pages` - you can't look through the class. – Gareth Latty Mar 05 '13 at 22:43
  • @Lattyware: He replaced `Pages` the class with `Pages` the instance. – Martijn Pieters Mar 05 '13 at 22:44
  • @MartijnPieters The constructor will run before that happens. – Gareth Latty Mar 05 '13 at 22:45
  • @Lattyware: Sure the constructor works before assignment to `Pages` takes place, but the `select()` method is not referenced from the constructor.. – Martijn Pieters Mar 05 '13 at 22:47
  • @MartijnPieters It will, but the constructor isn't where `select()` is run, so it doesn't matter - ignore me, you are right. That said, replacing the class with the instance is a horrible idea that makes the code hard to read. – Gareth Latty Mar 05 '13 at 22:48
  • Also, I doubt you are using 3.x due to the `object`s in your class declarations, but in 3.3+, you could replace `return iter(...)` with `yield from ...`. – Gareth Latty Mar 05 '13 at 22:52
  • I'm using 2.6 on embedded machine. I've replaced class name Pages to Website – Barmaley Mar 05 '13 at 23:33

4 Answers4

0

store a ref to the pageset and then loop through the pages in that set

class Page(object):
  def __init__(self, name, title, pageset):
    self.name   = name
    self.title  = title
    self.pageset = pageset
  def select(self):
    for page in self.pageset.pages:
        page.select = False
    self.select = True

class Pageset(object):
  def __init__(self):
    self.index    = Page("index", "Home", self )
    self.settings    = Page("settings", "Settings", self )
    self.pages = [ self.index , self.settings , ]

Note that this gives you the ability to have multiple Pagesets , and you're looking at the class instances. Your code above is referencing the classes themselves ( which there's really no reason to do ). i put self.pages as an attribute, because you might want to store other data in a pageset .

Jonathan Vanasco
  • 15,111
  • 10
  • 48
  • 72
  • Where is the `self` parameter to the `.select()` method? You are shadowing the method with an attribute of the same name. – Martijn Pieters Mar 05 '13 at 22:51
  • thanks. i was just typing this fast. it's a really awkward question. i kind of want to delete my answer - because the question itself seems a bit backwards. – Jonathan Vanasco Mar 05 '13 at 22:52
0

You need to rename the .select attribute or the .select() method, and add a self parameter to the method, but otherwise your code will work:

class Page(object):
    def __init__(self, name, title):
        self.name   = name
        self.title  = title
        self.selected = False

    def select(self):
        for Page in Pages:
            Page.selected = False
        self.selected = True

With those changes, your code works:

>>> Pages = Pages()
>>> Pages.index
<__main__.Page object at 0x10a0b6cd0>
>>> Pages.index.selected
False
>>> Pages.index.select()
>>> Pages.index.selected
True
>>> Pages.faq.select()
>>> Pages.index.selected
False

However, that is not to say that this is a good architecture design. I'd say you need to move the responsibility to select a page to the Pages class instead, an most of all avoid replacing the class Pages with a global instance Pages.

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
  • It works the only thing, it selects everything for some reason – Barmaley Mar 05 '13 at 23:00
  • Nope, not with the code in this answer it doesn't. See the demo session; when `faq` is selected, `.index.selected` is False. – Martijn Pieters Mar 05 '13 at 23:08
  • Use my definition of `Page`; everything else is the same as you had before. – Martijn Pieters Mar 05 '13 at 23:29
  • Got it working. My code is cumbersome so it took a bit to test it. It did not work at first because I forgot to change select to selected at a place in a code where I actually test this condition. Many thanks!!! As a side question, what would be the best approach to solve such a problem? (I've got more detail above) – Barmaley Mar 05 '13 at 23:55
0

I would argue the best solution here is to move your select() method to the Pages class:

class Pages(object):
    ...
    def select(self, target):
        for page in self:
           page.select = False
        target.select = True

To me, this seems like a more logical place for it to sit, and means that the Page instances don't need to know about the Pages instance they belong to.

It may even be better to simply have a selected attribute on the Pages instance that holds the selected page, rather than each Page knowing if it is selected, but that depends on design.

Gareth Latty
  • 86,389
  • 17
  • 178
  • 183
0

A Pages object manages a number of individual Page objects. It should be responsible for things that involve them all as a set such as, in your example, flagging a single page as selected and unflagging the others.

While you could potentially make each Page aware of the Pages object that owns it, doing too much of this could quickly lead to a tangled mess of spaghetti code.

P.S. - try coming up with some better names, even talking about having Page-s and a Pages is uncomfortable.

Sean McSomething
  • 6,376
  • 2
  • 23
  • 28