82

Change to lower once: text.lower() instead of t.lower() inside loop. Use t in vowels to check if the character is vowels. vowels = {...} can be replaced with dict.fromkeys('aeiou', 0) (See dict.fromkeys) Caution: Use this only if the value is immutable. >>> dict.fromkeys('aeiou', 0) {'a': 0, 'i': 0, 'e': 0, 'u': 0, 'o': 0} vowels = dict.fromkeys('...


21

First I would choose a different representation of the colors during the computation, to avoid the “expensive” COLORS-{a, b}).pop() operation, which is executed $$ (n-1) + (n-2) + \ldots + 1 = \frac{(n-1)n}{2} $$ times for an input string of length \$ n \$. It becomes simpler if we represent the colors as numbers instead, for example $$ \text{Red} ...


17

Efficiency I won't say much about efficiency - because without a clear use-case it will be hard to know whether possible changes would be worth the effort - but my main concern would be fact that you force a whole file into a string, of which you immediately produce a second copy. It would be nice to see a version which takes a stream of some description ...


15

Your suggestion is not really type-safe as you can still pass a key of the wrong type. Therefore I would just use a normal (string) key. But I would add a generic TryGet method which takes account of the type. The setter needs not to be generic. static class DictionaryExtensions { public static T Get<T>(this IDictionary<string, object> ...


15

Such a data structure already exists, and is called ILookup<TKey, TElement>. It can be created using the ToLookup extension. The ToLookup<TSource, TKey>(IEnumerable<TSource>, Func<TSource, TKey>) method returns a Lookup<TKey, TElement>, a one-to-many dictionary that maps keys to collections of values. A Lookup<TKey, ...


15

I'm afraid your sorting is in vain because the normal dictionary does not guarantee that the items will be enumerated in the same order as you added them: For purposes of enumeration, each item in the dictionary is treated as a KeyValuePair<TKey, TValue> structure representing a value and its key. The order in which the items are returned is ...


15

The current time complexity is \$O(N * M)\$ where \$N\$ is the number of quotes and \$M\$ is the number of bad words. For every single word in a quote you are iterating over all the bad words to check if there is a match. We can do better than that. What if you would initialize bad words as a set and would just lookup if a word is there - the lookup itself ...


14

I'm pretty sure there's no better way to do this but I thought a consensus on here might be nice. Let me know what you think. At first glance it seems to be ok but one could assume that he/she is always getting either the value or the default value. If one would get the default value for e.g if the dictionary is empty, why shouldn't he/she get the default ...


14

You could look at it in a different way, transform the wall of bricks to a wall of edges, in your example it would be 1, 3, 5 3, 4 1, 4 2 3, 4 1, 4, 5 The first, the third and the last row have an edge at 1, thus you can draw a line that crosses three bricks less than the height of the wall. You need to find the edge that appears the most. In this case it ...


14

This code is incorrect: re.compile(r'\b({0})\b'.format(w), flags=re.IGNORECASE) If w contains any metacharacters, then this will do the wrong thing. You don't want a badword list containing Mr. to match MRI, do you? Prefer instead: re.compile(r'\b({0})\b'.format(re.escape(w)), flags=re.IGNORECASE) Other, general comments: I agree with @alexce's ...


14

If you are going to use the collections module at all (as you did for collections.defaultdict), then why not use collections.Counter, which offers a most_common() method?


13

public Key(string unitID, int address, int comPort, int id) Clearly this class represents something much more specialized than an all-purpose "key". Name the type for what it stands for! private Tuple<string, int, int, int> _impl; The private field should be readonly. But why don't you have this instead? private readonly string _unitId; private ...


13

You are right, you should not use a list comprehension just for its side-effects. It is not very readable and only confuses the reader of the code. Instead, just make it for loops. And put it into a function, so it is re-usable: from collections import defaultdict def reverse_dict_of_lists(d): reversed_dict = defaultdict(list) for key, values in d....


12

for message in d.get('messages') or (): do_something() get returns None by default for missing keys. a or b evaluates to b when bool(a) == False. An empty list and None are examples of values that are false in a boolean context. Note that you cannot use this trick if you intend to modify the possibly empty list that is already in the dict. I'm using () ...


12

I can see no advantage of computing the hash value from Int(self.x) and Int(self.y). As you already noticed, truncating the floating point numbers to integers loses information and therefore causes hash collisions. CGFloat is (like all Swift number point types) Hashable, and its hashValue is just the integer with the same memory representation (as one can ...


12

The tool you need here is itertools.product: >>> import itertools >>> keys, values = zip(*config_overrides.items()) >>> experiments = [dict(zip(keys, v)) for v in itertools.product(*values)] >>> len(experiments) 3234 >>> experiments[1034] {'nhidden': 4, 'nodetype': 'lstm', 'loss': 'msle', 'opt': 'nadam', 'actfunc'...


12

Alright, I finally decided to actually review how you use C++ this because I want to practice structuring feedback for code structured like this. Be advised that I'm reviewing this from a specific lens, which is the one used in professional environments: Code should be written in a way to be read by other people. 1: Macros You should not be using macros ...


11

Also an alternative: if (!myList.ContainsKey(myKey)) { myList.Add(myKey, 0); } myList[myKey]++;


11

You're iterating through controlStrings 4 times (because you have 4 Where clauses). It might be better to rewrite this as a for loop: foreach (var c in controlStrings) { var cultureN = c.CultureN; if (cultureN.Contains("cati") && cultureN.Contains("en")) ... add c to CatiControlStrings; if (cultureN.Contains("web") && ...


11

You can use LINQ's Zip method. As usual, it's a bit slower than manually written code, but unless this is in a hot spot it rarely matters. The cost of Console.WriteLine vastly exceeds the cost of LINQ. foreach(var pair in collection1.Zip(collection2, Tuple.Create)) { Console.WriteLine("{0}, {1}", pair.Item1, pair.Item2); } You can replace Tuple.Create ...


11

Assuming the values under the id keys always match properties of currentAppswitches. If you can change currentAppswitches's class to accept an NSNumber object rather than a BOOL you have some convenient options. If currentAppswitches is KVO compliant you could write: for (NSDictionary *item in switchesArray) { id value = [item objectForKey:@"value"]; ...


11

First I second what Vince Panuccio stated in his answer I don't think this class should have a reason to exist. A key is just that, a key. If you have duplicate keys and duplicate values what you're essentially after is a grouping or a dictionary or with a set or list as its value. Bug alert This will break with an StackOverflowException public ...


11

Generating and handling exceptions is considered to be an expensive operation, so if(! x.ContainsKey()) is better. Yeah, the code example I see in MSDN uses try/catch but that's to illustrate the exception not advocate that as "best practice." Documentation I've read is pretty adamant about not throwing exceptions needlessly. And you don't need try/catch ...


11

Here are two ways you could represent a graph with weighted edges in Python: Represent a graph as a mapping from a node \$n\$ to a mapping from neighbouring node \$m\$ to the weight \$w\$ of the edge from \$n\$ to \$m\$: graph = { 0: {2: 4, 4: 60, 3: 23}, 1: {}, 2: {3: 4}, 3: {1: 10}, 4: {2: 15}, } Represent a graph as a pair of ...


11

You can use the fact that d.keys() and d.values() are always aligned. This gets rid of both the awkward way of constructing the values as well as the sorting of the keys. def dict_product(d): keys = d.keys() for element in product(*d.values()): yield dict(zip(keys, element)) The assignment of keys is not strictly necessary, but it avoids ...


10

It's not mandatory but it's a widely accepted guideline. Break it only if you have a good reason to do so. No, they don't. Class should be internal (default) and static (because it has not instance methods). Main() method should be private to make clear that it won't be called outside this class but it's not mandatory (someone prefers public because it's ...


10

A shorter version, functionally equivalent to yours: def dict_zip(*dicts): return {k: [d[k] for d in dicts] for k in args[0].keys()} That's assuming all dicts have the same keys, or more exactly, all dicts have at least all the keys present in the first dict. To make it more robust and handle cases when dicts don't have the same keys: def dict_zip(*...


10

Most C# developers will place opening braces on a new line. If you do this as well then you make it easier to read the code by others. complement and index should use var "type" as well twoSumSolution isn't really needed and is only adding noise to the code. I like to use if (bool == false) instead of if (!bool) because I can grasp it at first glance ...


10

Note it is much easier to read if you chop up the comprehension into blocks, instead of having them all on one line You could use unpacking to remove some usages of line.split >>> { ... int(k): v ... for line in data.split() ... for k, v in (line.split(':'),) ... } {41: 'n', 43: 'n', 44: 'n', 46: 'n', 47: 'n', 49: 'n', 50: 'n', 51: 'n', ...


10

You have too much logic in the dict comprehension: {int(line.split(":")[0]):line.split(":")[1] for line in data.split("\n") if len(line.split(":"))==2} First of all, let's expand it to a normal for-loop: >>> result = {} >>> for line in data.split("\n"): ... if len(line.split(":"))==2: ... result[int(line.split(":")[0])] = ...


Only top voted, non community-wiki answers of a minimum length are eligible