How to shorten the code? - Def()

What is the most effective method for rewriting this code in a manner that is both concise and elegant?

def xy():
    xset = {x for x in range(-4, 6)}
    yset = {y for y in range(-3, 4)}
    A = {(x, y) for x in xset for y in yset if 2*x - y == 0}
    B = {(x, y) for x in xset for y in yset if 3*x + y == 0}
    return A&B, A|B

print(xy())

Can’t get more concise than comprehensions. Trying to figure out more concise and “elegant” code is not effective use of resources.

A better question for you to answer: how can I write clear code that communicates intent and purpose to humans such that it results in increased maintainability.

Writing good code isn’t about cramming as much functionality into as few characters as possible. It is about communication, being readable and understandable. Especially if you get into area where future developers don’t necessarily have the depth of knowledge about an API you’re using just yet, they might just be starting to work on your code. One way to do that would be to use literate programming, for instance see https://jesterking.github.io/rhipy/create_pbr_material_with_textures.html , or even better read Physically Based Rendering: From Theory to Implementation for the ultimate example of best possible documentation and runnable code in one. There are of course other approaches to ensuring good communication with other developers, including future self.

As is sometimes said: writing code is communicating with readers, most likely with you yourself in the future.

6 Likes

I agree with Nathan.

I can only offer an opinion as to whether code is “concise and elegant” or not, after it’s been written and I’ve read it (i.e. not in advance). But some other things to consider:

Ranges are immutable sequences(*), and cannot have step=0. Therefore all the items yielded from a range are already unique, and storing them in a set (in {}s containing no :s - that’s a dict) does not deduplicate or change of them. For testing membership, a set could be a possible optimisation for O(1) lookup. But not here. Here, firstly the two sets’re being iterated over (not used to test for membership) and that’s O(n) each at best anyway. Secondly the implementation of a membership test for a range is probably faster than the hash algorithms of sets and dicts (not to mention the overhead of memory allocation from creating those more complex data structures on the heap).

All a range needs to test for membership of x is evaluate: start <= x < end and ((x - start) % step == 0).

tl;dr consider getting rid of the first two sets for simplicity. However in this case, I think that would actually make the A and B lines longer and less readable, so as the double for loop code is being repeated anyway, howabout:

def xy():
    xy_pairs = [(x, y) for x in range(-4, 6) for y in range(-3, 4)]
    A = {(x, y) for x, y in xy_pairs if 2*x - y == 0}
    B = {(x, y) for x, y in xy_pairs if 3*x + y == 0}
    return A&B, A|B

print(xy())

(*) In Python 3. In Python 2 range() returns a list, the equivalent type is xrange.

1 Like

Although I don’t completely get what you’ve said, the code has improved.

def xy():
    xyset = {(x, y) for x in range(-4, 6) for y in range(-3, 4)}
    A = {(x, y) for x, y in xyset if 2*x - y == 0}
    B = {(x, y) for x, y in xyset if 3*x + y == 0}
    return A&B, A|B

print(xy())

How has the code improved?

:joy: shortened

Shorter does not automatically equate better.

And even then you can go even shorter by using shortest possible variable names and getting rid of whitespace.

While code golf can be fun it is not a style of programming to aspire to in production environments.

Even fewer characters:

def q():
 r=range;s={(x,y) for x in r(-4, 6) for y in r(-3, 4)}
 A={(x,y) for x,y in s if 2*x-y==0}
 B={(x,y) for x,y in s if 3*x+y==0}
 return A&B, A|B
print(q())

That is 163 against the 209 of your latest.

Anyway, I still don’t see what you want with a shared root and combined roots of two hard-coded functions over hard-coded ranges.

edit: even shorter character wise, 158

def q(): r=range;s={(x,y) for x in r(-4, 6) for y in r(-3, 4)};A={(x,y) for x,y in s if 2*x-y==0};B={(x,y) for x,y in s if 3*x+y==0};return A&B,A|B
print(q())

Readable? Maintainable? Communicative? Not really.

3 Likes

No cap, your script looks dope! :grinning:

learned a new thing - Your code cannot be fully displayed on the laptop screen.

def f():
    r = range; # adding a ';' looks professional
    return {True if (x**2 + 5*x +6) > (2*x + 5*x + 9)
            else False for x in r(-10, 10)}
print(f())

This one involves far fewer operations:

union, intersection = set(), set()
for x in range(-4, 6):
    ya = 2 * x
    yb = -3 * x
    if ya == yb:
        intersection.add((x, ya))
    if -3 <= ya < 4:
        union.add((x, ya))
    if -3 <= yb < 4:
        union.add((x, yb))
print(union, intersection)

Do you mean your code requires less computing power or calculations?
What causes a script to run more quickly or slowly? (From line 1 to 10, for example)

Here’s my concept:
If there are more functions in a script, such as methods() and def(), which take longer to compute. Strings are likely to require less time to compute because they are one-to-one functions.

I can understand your code, but writing it in your style takes longer because there is a mental step of ‘adding back to something.’ If your script is faster, I will definitely modify my style for writing code. Is it true, though?

union, intersection = set(), set() # it looks professional...

My code runs for 10 values of x, creating 2 y variables and evaluating 3 if conditions per iteration.
My code only evaluates points which lie on your two lines.

your code does this:

def xy():
    xset = {x for x in range(-4, 6)}  # set of 10 values
    yset = {y for y in range(-3, 4)}  # set of 7 values
    A = {(x, y) for x in xset for y in yset if 2*x - y == 0}  # evaluates 70 pairs of (x, y)
    B = {(x, y) for x in xset for y in yset if 3*x + y == 0}  # the same 70 pairs of (x, y) again
    return A&B, A|B

print(xy())

Your code evaluates every point in the 2D space you are considering - you could argue that my code is more effective because it operates on fewer irrelevant points. I would expect my code to scale better to larger spaces (if you correct the logical error where I fail to check that the intersection lies inside the allowed bounds of y values).

You’re right :smiley:

1 Like

leading me to this:


A = {(x, 2*x) for x in range(-4, 6) if -3 <= 2*x < 4}
B = {(x, -3*x) for x in range(-4, 6) if -3 <= -3*x < 4}

print(A.union(B), A.intersection(B))
1 Like

It is not to look professional, but to be able to put multiple statements on one line.

To show the speed difference between the optimized and non-optimized versions, running each version a million times:

6-7 times faster, very nice!

1 Like

I think I found the most readable solution. Its a one-liner so I think this is the most convenient and elegant way. I encourage other developers to write more code like this. Makes life easier for me!

xy =lambda:eval(''.join([chr(ord(_)-0b01) for _ in \
    "fwbm)czuft/gspnify)(398C39413D31413:8E3D318C39423D31433:3D31393E423D31443:3D31393E423D313E433:3D3139423D313E443:3D3139413D31413:8E3:(*/efdpef)**"]))
print(xy())
2 Likes

So ‘readable’, one must be concerned about malware.

That firstly decrpyts to:

eval(bytes.fromhex('287B28302C2030297D2C207B28312C2032292C20282D312C2033292C20282D312C202D32292C2028312C202D33292C2028302C2030297D29').decode())

and then inside the inner eval those bytes decode to:

'({(0, 0)}, {(1, 2), (-1, 3), (-1, -2), (1, -3), (0, 0)})'
1 Like

This thread is about obfuscation, isn’t it? Rule number 1, execute first and ask questions later :wink: