Open-event-server: empty_string() needs a rewrite

Created on 20 Jul 2019  路  8Comments  路  Source: fossasia/open-event-server

Describe the bug

https://github.com/fossasia/open-event-server/blob/development/app/api/helpers/utilities.py#L28-L34 is exceedingly convoluted, does __type(x)__ compares contrary to PEP8, and contains several logic errors. A code review of this function would be a worthwhile effort. Some __doctests__ would really be helpful here.

To Reproduce
Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior

Stacktrace

Additional details (please complete the following information):

  • OS: [e.g. MacOS, Ubuntu, CentOS]
  • Python Version [e.g. 3.5, 3.6]
  • HEAD Commit hash [e.g. 4629c62]

Additional context

bug

All 8 comments

Issue-Label Bot is automatically applying the label bug to this issue, with a confidence of 0.71. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

Yup. it was created when Python 2 to 3 migration/compatibility was needed

There are no doctests, but I think it has some unit tests. If not, they'll be added

There are unit tests. The behaviour is flaky though. None should return True, it is returning False

https://github.com/fossasia/open-event-server/blob/ff408e557f6ff12e37e72a5201d110d589ce50e3/tests/all/integration/api/helpers/test_utilities.py#L32

Perhaps add a few more tests for clarity:

self.assertTrue(string_empty('  '))  # <space><space>
self.assertTrue(string_empty(' ' * 20))  # <space> * 20
self.assertFalse(string_empty(0))  # an int
self.assertFalse(string_empty(0.0))  # a float
self.assertFalse(string_empty([]))  # a list
self.assertFalse(string_empty(False))  # a bool
self.assertTrue(string_empty('\t  \n'))  # <tab><space><return>

Does this codebase still support Python 2 or not?

From README.md it looks like legacy Python can be dropped so:

def string_empty(value: str) -> bool:
    if isinstance(value, str):
        return not value.strip()
    return False
def string_empty(value: str) -> bool:
    return isinstance(value, str) and not value.strip()
Was this page helpful?
0 / 5 - 0 ratings

Related issues

mariobehling picture mariobehling  路  4Comments

rafalkowalski picture rafalkowalski  路  3Comments

Masquerade0097 picture Masquerade0097  路  3Comments

CosmicCoder96 picture CosmicCoder96  路  4Comments

aditya1702 picture aditya1702  路  4Comments