6
\$\begingroup\$

Problem Statement:

Consider the following example: a coin was tossed n=7 times and the results were

"Heads, Tails, Tails, Tails, Heads, Heads, Tails"

Therefore, the longest Heads streak was 2 and the longest Tails streak was 3.

Here is my code:

def getMaxStreaks(toss): #toss=> ["Heads, Tails, Tails, Tails, Heads, Heads, Tails"]
    # Return an array of two integers containing the maximum streak of heads and tails respectively
    scH, scT = 0, 0
    H, T = 0, 0
    for tossResult in range(0, len(toss)):
        if toss[tossResult] == 'Heads':
            H, T = H+1, 0
            if H > scH:
                scH = H

        elif toss[tossResult] == 'Tails':
            H, T = 0, T+1
            if T > scT:
                scT = T
    return [scH, scT] # 2 3

What can I do to improve my code, are there any improvements required? Please let me know if I need to update the question.

\$\endgroup\$
7
\$\begingroup\$

Don't iterate over the indices of a list only to then access the element at that index. Instead iterate directly over the elements of the list! Have a look at Loop Like A Native by Ned Halder.

In the standard library module itertools there is the groupby function, which groups equal elements together. You can use this to make your code easier:

from itertools import groupby

def max_streaks(tosses):
    longest_streaks = {"Heads": 0, "Tails": 0}
    for toss, streak in groupby(tosses):
        longest_streaks[toss] = max(longest_streaks[toss], len(list(streak)))
    return longest_streaks["Heads"], longest_streaks["Tails"]


max_streaks("Heads, Tails, Tails, Tails, Heads, Heads, Tails".split(", "))
# 2, 3

Note that this does not assume anything about tosses, unlike your code. It could be a list or a string, but also something like a generator, which has no len.

You could generalize this function to being able to count the longest streak of any elements by using a collections.defaultdict and returning it:

from collections import defaultdict
from itertools import groupby
from typing import Dict, Iterable, Hashable

def longest_streaks(elements: Iterable[Hashable]) -> Dict[Hashable, int]:
    """Count the length of the longest streak of each distinct element 
    present in `elements`.
    All elements need to be hashable.
    """
    longest_streak = defaultdict(int)
    for element, streak in groupby(elements):
        longest_streak[element] = max(longest_streak[element], len(list(streak)))
    return dict(longest_streak)


if __name__ == "__main__":
    longest_streaks([1, 2, 2, 3, 3, 3, 1, 1, 1, 2, 2, 3])
    # {1: 3, 2: 2, 3: 3}

Note that I followed Python's official style-guide, PEP8, by using lower_case for variables and functions and wrapped the calling code in a if __name__ == "__main__": guard to allow importing from this script without running the example. I also added some type hints and a docstring, which makes it easier to figure out what a function does, both for anybody else reading your code and for yourself in two months.

| improve this answer | |
\$\endgroup\$
5
\$\begingroup\$

On top of @Graipher's style recommendations, you should use better, more descriptive names to begin with. In this context, it's fairly clear what at least part of the intent of H and T are. H has to do with "heads"... but what about "heads"? I would name it something closer to head_streak, or something else descriptive. Especially with as short of lines as you have, succinct variables names aren't necessary or even preferable. And that suggestions applies to an even greater extent for names like scH. Be descriptive. record_heads or something similar would be better.


I'm not a fan of multiple variable assignments on one line; especially when it's using tuple destructuring.

H, T = H+1, 0

That may be succinct and fit on one line, but I don't think it reads near as clearly as:

H += 1
T = 0

And I would say the same about

scH, scT = 0, 0

And the like at the top.


Arguably, it would be better to have the function return a tuple instead of a list. Really, it doesn't make any actual difference. Symbolically though, you're returning a structure that will always have exactly two elements. If you were to type-hint your function signature:

from typing import List

def getMaxStreaks(tosses: List[str]) -> List[int]:
    . . .

That, what you have now, suggests that the function could return a variable amount of numbers, which isn't true. This on the other hand though:

from typing import List, Tuple

def getMaxStreaks(tosses: List[str]) -> Tuple[int, int]:
    . . .

Lets the caller (and the IDE) know that the function will always return exactly two integers.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ sure, thanks for your answer. I will explore how to specify return value for a function. \$\endgroup\$ – Bhansa Mar 16 at 6:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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