Crossopt 1 #4
Crossopt 1 #4
Conversation
crossopt/assignment1/Q1.py
Outdated
|
|
||
|
|
||
| # countsorts elements in container, elements must be hashable | ||
| def countSort(container): |
There was a problem hiding this comment.
I like this approach, it is efficient!
There is another callable in the standard library that already does this:
https://docs.python.org/2/library/collections.html#collections.Counter
Can you use it?
collections is very cool Python module, I suggest you read it all, ask me questions if you do not know why/when these data structure would be helpful. Fast efficient datastructures, it shines in interviews ;)
One last thing: the name countSort is a peculiar choice: it does not sort! A dict is an unordered container.
crossopt/assignment1/Q1.py
Outdated
|
|
||
|
|
||
| # checks whether strings are anagrams | ||
| def isStringAnagram(str1, str2, caseSensitive=False): |
There was a problem hiding this comment.
Please use the Python name convention:
https://www.python.org/dev/peps/pep-0008/#function-and-variable-names
A bit extended, the one we use at Google:
https://google.github.io/styleguide/pyguide.html
crossopt/assignment1/Q1.py
Outdated
| str1 = str1.lower() | ||
| str2 = str2.lower() | ||
|
|
||
| punctuation = '[! "#$%&()*+,\./:;<=>?@[\\]^`{|}~]' |
There was a problem hiding this comment.
To make it easier for you, and also more standard you can use the character class in the re module:
https://docs.python.org/3/library/re.html#regular-expression-syntax
I think the character class you can use is '\W'
The re character class have the benefit of working on unicode strings as well.
There was a problem hiding this comment.
I thought about using \W, but I don't think everything Python considers a word boundary is one in the English language.
Like apostrophes, for example. Treating don't like two words really doesn't sound right.
And in the end, I decided that being able to see exactly which characters I considered word boundaries inside the code made more sense than tricky regexes
crossopt/assignment1/Q1.py
Outdated
| # get list of words in sentence | ||
| words1 = [word for word in split(punctuation, str1) if word] | ||
| # replace all words with their CountSort, make it hashable | ||
| words1 = [(tuple(sorted(countSort(word).items()))) for word in words1] |
There was a problem hiding this comment.
Are you sure the 'tuple' call is needed? sorted returns a list and the list comprehensions ends up making a list of list?
There was a problem hiding this comment.
Well, list isn't hashable, so I do need some cast.
words is a list of the words in the original sentence, I want to end up with a list of tuples that define the word and countSort them
As to whether casting the list to tuple is better than to some other type like str, I really have no idea
crossopt/assignment1/Q1.py
Outdated
| # replace all words with their CountSort, make it hashable | ||
| words1 = [(tuple(sorted(countSort(word).items()))) for word in words1] | ||
| words2 = [word for word in split(punctuation, str2) if word] | ||
| words2 = [(tuple(sorted(countSort(word).items()))) for word in words2] |
There was a problem hiding this comment.
I see that the logic for creating a comparable representation from the sentence is duplicated, does it make sense to encapsulate in its own function?
It is then easier to fix or refactor, harder to make mistakes by having both processing out of sync.
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| str1 = "triangle" |
There was a problem hiding this comment.
Unit test: they are not mandatory for the first assignment, but I will ask for unit tests for the second assignment. If you want to get started with unit test, read the page below and copy/paste the snippet of code in the section "Basic Examples"
https://docs.python.org/3/library/unittest.html
There are many practical benefits to unit tests, for you author of the code, and especially for the team of colleagues working with you. You will be more productive with knowing how to do unit tests.
Ping me if you want to know more about test or if you have any questions.
crossopt/assignment1/Q2.py
Outdated
| new_node.next = self.head | ||
| self.head = new_node | ||
|
|
||
| def length(self): |
There was a problem hiding this comment.
I see below that you know about the str() that works with print.
Here in the same vein, you can use len() instead of length() to have the Python builtin len() function that still works.
crossopt/assignment1/Q2.py
Outdated
| return "{" + ", ".join(ans) + "}" | ||
|
|
||
| # return kth element from end of list | ||
| def getKth(self, k): |
There was a problem hiding this comment.
Do you think you can make the bracket notation work?
This means means that your container linkedlist would work with the syntax:
llist = LinkedList()
llist.insert(5)
llist.insert('dog')
llist.insert(set([1, 2, 3]))
llist[-1], llist[-2], llist[-3]
If you use the Pythonic convention like bracket for accessing an element by its index, the benefit is for your users: they can quickly learn how to use your data structure without a lot of effort, because the syntax is the same as for most other container like dictionary or lists.
|
|
||
|
|
||
| class Node: | ||
| class Node(object): |
There was a problem hiding this comment.
The "(object)" is necessary in Python2, but if you write Python 3, then it is not needed.
| for i in range(length + k): | ||
| cnt = cnt.next | ||
| return cnt | ||
|
|
There was a problem hiding this comment.
For the second assignement, I will require that you use unit tests :)
You can take a look at my pull request or the pull request of the other mentees:
Laida: 14499bc
JD: https://github.com/flerdacodeu/CodeU-2018-Group2/pull/10/files
No description provided.