Skip to main content
Post Undeleted by Peilonrayz
added 136 characters in body
Source Link
Peilonrayz
  • 44.6k
  • 7
  • 80
  • 158
  • The variable names in change_char_at are terrible. s for the string, ch for the char? i for the start? j for the end.

    Seriously just write out names.

    def change_char_at(string: str, change: str, start: int, end: int = None):
        return string[:start] + change + string[end or start+1:]
    
  • Your code isn't fully typed:

    • change_char_at has no return type.
    • end in change_char_at and wait in loader_with_wait are assumed to be Optional but not specified to be optional.
    • loader and loader_with_wait have no return types should they be None or NoReturn?
  • The functionallity of change_char_at can be eclipsed by the format mini-language.

    1. You have a standard structure where the bar is surrounded by [] and filled with the contents.

      bar = '[{}]'
      
    2. The content of your bar is left aligned by width.

      bar = '[{: <{width}}]'
      
    3. Each iteration you increase the contents is increasedleft hand space by 1 equals.

      bar.format('='' ' * (i + '=' * bar_width), width=width)
      
  • bar_width doesn't actually change the size of the bar.

  • I would prefer the progress bar to be a generator function or list, so the rest of your code is simpler.

import itertools


def progress_bar(width: int = 20, bar_width: int = 3):
    bar = f'[{{: <{width}}}]'
    for i in itertools.chain(
        range(0, width - bar_width + 1),
        reversed(range(0, width - bar_width)),
    ):
        yield bar.format('='' ' * i + '=' * bar_width)
  • The variable names in change_char_at are terrible. s for the string, ch for the char? i for the start? j for the end.

    Seriously just write out names.

    def change_char_at(string: str, change: str, start: int, end: int = None):
        return string[:start] + change + string[end or start+1:]
    
  • Your code isn't fully typed:

    • change_char_at has no return type.
    • end in change_char_at and wait in loader_with_wait are assumed to be Optional but not specified to be optional.
    • loader and loader_with_wait have no return types should they be None or NoReturn?
  • The functionallity of change_char_at can be eclipsed by the format mini-language.

    1. You have a standard structure where the bar is surrounded by [] and filled with the contents.

      bar = '[{}]'
      
    2. The content of your bar is left aligned by width.

      bar = '[{: <{width}}]'
      
    3. Each iteration the contents is increased by 1 equals.

      bar.format('=' * (i + bar_width), width=width)
      
  • bar_width doesn't actually change the size of the bar.

  • I would prefer the progress bar to be a generator function or list, so the rest of your code is simpler.

def progress_bar(width: int = 20, bar_width: int = 3):
    bar = f'[{{: <{width}}}]'
    for i in range(bar_width, width):
        yield bar.format('=' * i)
  • The variable names in change_char_at are terrible. s for the string, ch for the char? i for the start? j for the end.

    Seriously just write out names.

    def change_char_at(string: str, change: str, start: int, end: int = None):
        return string[:start] + change + string[end or start+1:]
    
  • Your code isn't fully typed:

    • change_char_at has no return type.
    • end in change_char_at and wait in loader_with_wait are assumed to be Optional but not specified to be optional.
    • loader and loader_with_wait have no return types should they be None or NoReturn?
  • The functionallity of change_char_at can be eclipsed by the format mini-language.

    1. You have a standard structure where the bar is surrounded by [] and filled with the contents.

      bar = '[{}]'
      
    2. The content of your bar is left aligned by width.

      bar = '[{: <{width}}]'
      
    3. Each iteration you increase the left hand space by 1.

      bar.format(' ' * i + '=' * bar_width, width=width)
      
  • bar_width doesn't actually change the size of the bar.

  • I would prefer the progress bar to be a generator function or list, so the rest of your code is simpler.

import itertools


def progress_bar(width: int = 20, bar_width: int = 3):
    bar = f'[{{: <{width}}}]'
    for i in itertools.chain(
        range(0, width - bar_width + 1),
        reversed(range(0, width - bar_width)),
    ):
        yield bar.format(' ' * i + '=' * bar_width)
Post Deleted by Peilonrayz
Source Link
Peilonrayz
  • 44.6k
  • 7
  • 80
  • 158

  • The variable names in change_char_at are terrible. s for the string, ch for the char? i for the start? j for the end.

    Seriously just write out names.

    def change_char_at(string: str, change: str, start: int, end: int = None):
        return string[:start] + change + string[end or start+1:]
    
  • Your code isn't fully typed:

    • change_char_at has no return type.
    • end in change_char_at and wait in loader_with_wait are assumed to be Optional but not specified to be optional.
    • loader and loader_with_wait have no return types should they be None or NoReturn?
  • The functionallity of change_char_at can be eclipsed by the format mini-language.

    1. You have a standard structure where the bar is surrounded by [] and filled with the contents.

      bar = '[{}]'
      
    2. The content of your bar is left aligned by width.

      bar = '[{: <{width}}]'
      
    3. Each iteration the contents is increased by 1 equals.

      bar.format('=' * (i + bar_width), width=width)
      
  • bar_width doesn't actually change the size of the bar.

  • I would prefer the progress bar to be a generator function or list, so the rest of your code is simpler.

def progress_bar(width: int = 20, bar_width: int = 3):
    bar = f'[{{: <{width}}}]'
    for i in range(bar_width, width):
        yield bar.format('=' * i)