Skip to content

PR to add some hash problems#387

Merged
goswami-rahul merged 5 commits into
keon:masterfrom
danghai:map
Aug 14, 2018
Merged

PR to add some hash problems#387
goswami-rahul merged 5 commits into
keon:masterfrom
danghai:map

Conversation

@danghai
Copy link
Copy Markdown
Collaborator

@danghai danghai commented Aug 5, 2018

(Put an X inside the [ ] to denote check mark [X].)

  • If creating a new file :

    • added links to it in the README files ?
    • included tests with it ?
    • added description (overview of algorithm, time and space compleixty, and possible edge case) in docstrings ?
  • if done some changes :

    • wrote short description in the PR explaining what the changes do ?
    • Fixes #[issue number] if related to any issue
  • other

Copy link
Copy Markdown
Collaborator

@goswami-rahul goswami-rahul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 👍

Comment thread algorithms/map/is_isomorphic.py Outdated
map = {}
for i in range(len(s)):
if s[i] not in map:
if t[i] in map.values():
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make this O(n) from O(n^2), if we store the map.values() in a seperate set.

Comment thread algorithms/map/is_isomorphic.py Outdated
"""
if len(s) != len(t):
return False
map = {}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

map overshadows built-in map in Python.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean changing the name?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep good catch

Comment thread algorithms/map/word_pattern.py Outdated
return False
for i in range(len(pattern)):
if pattern[i] not in map:
if list_str[i] in map.values():
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar optimization can be used here too!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a good idea

Comment thread algorithms/set/find_keyboard_row.py Outdated
:rtype: List[str]
"""
keyboard = [
set(list('qwertyuiop')),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think set('qwertyuiop') will be fine. No need for list.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>>> set('acvvvv')
set(['a', 'c', 'v'])

yep

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I will fix now

@danghai
Copy link
Copy Markdown
Collaborator Author

danghai commented Aug 8, 2018

@goswami-rahul @keon I have updated!

@coveralls
Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 715

  • 62 of 65 (95.38%) changed or added relevant lines in 6 files are covered.
  • 5 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.6%) to 71.426%

Changes Missing Coverage Covered Lines Changed/Added Lines %
algorithms/map/word_pattern.py 14 15 93.33%
algorithms/map/is_isomorphic.py 12 14 85.71%
Files with Coverage Reduction New Missed Lines %
algorithms/sort/top_sort.py 2 90.38%
tests/test_backtrack.py 3 95.92%
Totals Coverage Status
Change from base Build 712: -0.6%
Covered Lines: 4297
Relevant Lines: 6016

💛 - Coveralls

@goswami-rahul goswami-rahul merged commit b02e3bf into keon:master Aug 14, 2018
@keon
Copy link
Copy Markdown
Owner

keon commented Aug 14, 2018

nice work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants