current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite 
from random import random

def to_color(obj=None):
    """Determine a color (string 000000-ffffff) from the hash of any object

    If no argument is passed, a random color is returned.

    Args:
        obj: any hashable object; string, tuple, ...

    Returns:
        a color string in range '000000' to 'ffffff'
    """
    obj = obj if obj else random()
    return "{0:06x}".format(abs(hash(obj)))

My questions:

  1. Is the line obj = obj if obj else random() idiomatic?
  2. Is hash(obj) the "proper" way to get a deterministic yet scrambled string for my purpose?
  3. The abs takes care of negative hash values (e.g. from hashing negative integers). Is there a better way which doesn't collide so easily for small (absolute) integer values? (Right now, to_color(500) is equal to to_color(-500).)
share|improve this question

1 Answer 1

up vote 4 down vote accepted

From PEP 8:

Comparisons to singletons like None should always be done with is or is not, never the equality operators.

Also, beware of writing if x when you really mean if x is not None -- e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context!

What you should do in your case is : obj = obj if obj is not None else random().

Then, even though I like the ternary operator, I reckon its usage is not required/idiomatic here. The usual way to handle default parameter is more like

if arg is None:
    arg = default()

As far as I can tell, using hash like you did is ok. However, I guess you should handle values that could be out of the range you are expecting (I couldn't find easily something telling which range of values can be returned).

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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