Path: csiph.com!usenet.pasdenom.info!gegeweb.org!usenet-fr.net!nerim.net!novso.com!newsfeed.xs4all.nl!newsfeed3.news.xs4all.nl!xs4all!newsgate.cistron.nl!newsgate.news.xs4all.nl!post.news.xs4all.nl!not-for-mail
MIME-Version: 1.0
In-Reply-To: <dd384c6d-17c0-4850-ad5b-6b073c80bdb8@googlegroups.com>
References: <dd384c6d-17c0-4850-ad5b-6b073c80bdb8@googlegroups.com>
Date: Tue, 13 Jan 2015 08:40:49 +1100
Subject: Re: Broken IF statement
From: Chris Angelico <rosuav@gmail.com>
Cc: "python-list@python.org" <python-list@python.org>
Content-Type: text/plain; charset=UTF-8
Precedence: list
Newsgroups: comp.lang.python
Message-ID: <mailman.17641.1421099337.18130.python-list@python.org>
Lines: 78
NNTP-Posting-Host: 2001:888:2000:d::a6
Xref: csiph.com comp.lang.python:83637

On Tue, Jan 13, 2015 at 8:19 AM,  <joboppsgpp@gmail.com> wrote:
> https://bpaste.net/show/93be9e15634b <--- Line 19 through 22
>
> At all times, my program is assigning the object priority of 0, even if one already exists in the database with a priority of 0 (it's supposed to be assigning it a priority of 1 in those cases).
>
> I'm a non developer trying to fix a freelancer's code. Would anybody be able to suggest changes to the IF logic that might be able to fix it, assuming the statements in the code provided look flawed?

Normally, I would suggest talking to the freelancer who wrote the
code, unless you're no longer working with him/her. Changing someone's
code out from under them is a great way to annoy and confuse.

Including the text in-line as it's short enough for that:

def create_socialaccount(profile, urls, twitters, facebooks, statuses=None):
    """
    Add or update all the social accounts linked to the 'profile'
    """
    results = []
    l = [(urls, SocialAccount.HOMEPAGE, HOMEPAGE_COL_START),
         (twitters, SocialAccount.TWITTER, TWITTER_COL_START),
         (facebooks, SocialAccount.FACEBOOK, FACEBOOK_COL_START)]
    for objs, service, offset in l:
        for i, value in enumerate(objs):
            if value:
                obj, created = SocialAccount.objects.get_or_create(
                    social_profile=profile,
                    service=service,
                    value=value,
                )
                # The first object added/updated gets a priority of 0, all
                # others get a 1
                if i == 0:
                    obj.priority = 0
                else:
                    obj.priority = 1


What kind of object is this 'obj'? After you make a change to it, do
you need to tell it to write to a database or something?

What you could try is changing the priority assignments to, say, 2 and
3. That would tell you that it's making the change. But if the
intention is to have the first successful one at priority 0 and all
others at priority 1 (which is what the comment implies), then I'd
write it like this:

def create_socialaccount(profile, urls, twitters, facebooks, statuses=None):
    """
    Add or update all the social accounts linked to the 'profile'
    """
    l = [(urls, SocialAccount.HOMEPAGE, HOMEPAGE_COL_START),
         (twitters, SocialAccount.TWITTER, TWITTER_COL_START),
         (facebooks, SocialAccount.FACEBOOK, FACEBOOK_COL_START)]
    for objs, service, offset in l:
        prio = 0
        for value in objs:
            if value:
                obj, created = SocialAccount.objects.get_or_create(
                    social_profile=profile,
                    service=service,
                    value=value,
                )
                # The first object added/updated gets a priority of 0, all
                # others get a 1
                obj.priority = prio
                prio = 1

It's not clear whether "first" means "first of each type" or "first
overall". For instance, if someone has three twitters and two
facebooks, should one twitter and one facebook be given priority 0, or
should one twitter get prio 0 and everything else prio 1? I've coded
it for the former, but you could easily make it the latter by simply
shifting the "prio = 0" statement one line further up (and unindenting
it), thus putting it before the entire loop.

Does that help, at all?

ChrisA