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} ...


21

Recursive Approach Review In this method you are performing too many look-ups. First, you are looking up the 0th value of p two or three times. List indexing does take time, so it may be faster to store p[0] in a local variable, to avoid repeatedly indexing into the list. Second, you are performing a dictionary looking ups two or three times. While ...


18

It's standard to indent Python with 4 spaces. Not following this standard only makes your life and the life of people that have to interact with you harder. You can just use the optional default argument to dict.get to remove the need for the if. for k1, v1 in dict1.items(): for k2, v2 in dict2.items(): key = k1 + k2 dict_merged[key] = ...


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 ...


16

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 ...


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 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 ...


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 ...


15

PEP-8 Class names should be CapWords, so instead of rectangle you should have Rectangle. Commas should be followed by 1 space. You've mostly followed this, except in s1 = rectangle({'p1': (1,1), 'p2': (2,2)}) Bugs The formula for "area" is not twice the sum of width & height. I don't know what "surrounding" is, but the formula for perimeter is not ...


14

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 () ...


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

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'...


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

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....


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

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 ...


13

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(*...


13

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 ...


12

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 ...


12

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"]; ...


12

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 ...


12

Use a library. In particular, glom does an admirable job of this, and is actively developed. >>> d = {"a":{"1":{"aa":"a2", "aaa":"a3"}}, ... "b":{"2":{"bb":"b2", "bbb":"b3"}}, ... "c":{"3":{"cc":"c2", "ccc":"c3"}}, ... "d":{"4":{"dd":"d2", "ddd":"d3"}}, ... "e":{"5":{"ee":"e2", "eee":"e3"}} ... } >>> import glom &...


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

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

Unless I'm missing something you'd like to implement, I would go for a simple loop instead of using recursion: def nested_get(input_dict, nested_key): internal_dict_value = input_dict for k in nested_key: internal_dict_value = internal_dict_value.get(k, None) if internal_dict_value is None: return None return ...


11

When reading the title of the question, I wondered: "Why use a dict subclass when a regular class with __slots__ can do?". And judging by the interface, it seems that it may fit. Slots defines attributes names that are reserved for the use as attributes for the instances of the class. However, no more attributes can be added to the instances when defining ...


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