# Tag Info

45

You get the job done in 30 minutes and the use of a stack is the way to go, so that's a good start. In my opinion you're writing a little too much (repetitive) code and it could be a lot easier to read if you use a switch-statement instead: public bool IsValidReview(string s) { Stack<char> endings = new Stack<char>(); foreach (var curr in s)...

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

This is a follow up of @Henrik Hansen. Instead, of a switch I would use a Dictionary<T, K>. A Dictionary offers two main advantages: an increase readibility and the suppression of every magic string from your function. public static readonly Dictionary<char, char> brackets = new Dictionary<char, char> { {'(', ')'}, {'[', ']'}, {...

13

There are couple of things that might have been done better... Initilize the dictionary form a static constructor or call the method to initialize the field. The method should not access it internally but return a dictionary as a result. The field itself should be readonly. foreach could be used to iterate the stringToEncode. Names of internal variables ...

11

A couple of small things to add: Maybe not applicable to a timed interview, but inline documentation (///) on public members is always nice, and would help to explain the otherwise vague IsValid method name. I'd want to throw an exception if any other character is encountered, since the behaviour is undefined and undocumented. The spec says to assume only ()...

11

def validateIP(ip): I would expect a name starting is (a useful hint that it returns a Boolean rather than some more complex validation data structure) and explicitly mentioning IP v4 (since the current name is misleading). E.g. is_valid_IPv4_address. #split them by '.' , and store them in an array #check the array if the length is 4 length arr = ip....

9

public static ReadOnlyDictionary<int, int[]> AvailableMoveIndexes => new ReadOnlyDictionary<int, int[]> ( new Dictionary<int, int[]> { [0] = new[] { 1, 2 }, ... Nice clean representation, but why public? I would expect private or (perhaps better) internal with an InternalsVisibleTo attribute ...

8

There are a few things you can do to simplify your code: There's no need to keep track of the maximum positive integer if you remove the condition from the second for loop: for (int i = 1; ; i++). That means you don't need to check whether the hash already contains the given number: just add it right away. If you don't mind a small performance hit you can ...

8

There's a bug: you're not checking if the last character is a letter or digit, only that it isn't a hyphen, so this fails to reject "abcdef&". Denis' solution may be less efficient (2 iterations instead of 1), but with at most 16 characters that's not much of a concern, and it's both easier to read and it works correctly. The first two rules read very ...

8

You are using one explicit pass over the list (for i in list), and one implicit pass over the list (k - i in list). This means your algorithm is $O(N^2)$. Note: You can reduce your implementation to one line: return any(k - i in list for i in list) But you have a bug. is_sum_of_2nums_in_list_k([5], 10) returns True. A one pass algorithm starts with ...

8

General comments There is no reason to use a class. Instead, the functionality should be made into a free function. Your code is overly complicated. There is no reason to make new string from which you erase characters one-by-one. Instead, you can convert the input integer to a string and use a standard function to reverse that. Also, pay attention to const ...

8

A doc string reads nicer then # blockcomments Consider making a doc string of that function, so you can do help(validate_ip) and it will print the doc string in the interpreter. Adhere to PEP8 Functions and variables should be snake_case ie def validate_ip(ip): You could use the all keyword to check if each part is correct; this will return False for the ...

7

Additional to @t3chb0t real code review, I would like to provide an alternative more object oriented implementaion. Not sure if that solution is really better / more readable / appropriate for such a small problem - but at least it follows the SOLID principles: public class InputProcessor { private static readonly ICharStreamProcessor[] ...

7

Review Add docstrings and tests... or both in the form of doctests! def find_word(s, index): """ Finds the CamalCased word surrounding the givin index in the string >>> find_word('CamelCaseString', 6) 'Case' >>> find_word('ACamelCaseString', 0) 'A' """ ... Loop like a native. Instead of going over the ...

7

Not much to say about the extensions methods, as they are mostly wrappers. However if you're looking for ways to make the algorithm more readable, LINQ is your friend. You can replace most of your logic with a one-liner: var hyphenCount = 0; for (var i = 1; i < str.Length - 1; i++) { if (str[i].IsLetterOrDigit()) { continue; } ...

7

There are two points that showed up immediately when looking at your implementation: You missed one of the goals of the task: It should read the data from standard input and writes it the result to standard output. You are reading in the complete binary data into memory and process it from there. The latter is a no go, if we're talking about large ...

7

Just a few things to add to VisualMelons answer: Instead of a for loop, you can use a foreach loop: foreach (string word in Value) { ... Using Dictionary<string,int>.TryGetValue: The out int count value defaults to 0 if the word is not present in the dictionary, so it's valid to write: foreach (string word in Value) { ...

7

There are some basic considerations to make in your design. Guard arguments Perform at least NotNull checks on arguments on public entrypoints of your API. public OrderDeliverer(Warehouse warehouse, IOrderStreamer orderStreamer) { this.warehouse = warehouse; this.orderStreamer = orderStreamer; } public OrderDeliverer(Warehouse ...

7

I'll try to cover the response part. SingleProduct and BundleProduct should be polymorphic. ConcretePrice, DiscountedPriceByAmount, DiscountedPriceByPercentage should be polymorphic. I think they are talking about the following: you need to have a table of products and a table of prices this is not exactly a requirement, but I guess this would be a ...

7

Avoid magic numbers, especially if they, like 26 in your code, are used repeatedly. Along the same line, keep in mind that the code only works in the "C" locale. Other locales may have alphabets of different size. Prefer to declare variables as close to use as possible, e.g. for (size_t i = 0; i < l1; ++i) { int index1 = s1[i] - 'a'; ...

7

Your code Your code as such seems to be functional, but not really elegant or concise. First, the variable names don't speak for themselves. Nobody would be hurt if the function input was named numbers instead of a and number instead of ai. work_dict is also not a particularly good name since it's very generic. How about digit_histogram? Handling single ...

7

First of all, whenever you see this ... this is a big no-no: '9'*work_dict[9] + '8'*work_dict[8] + '7'*work_dict[7] + '6'*work_dict[6] + '5'*work_dict[5] + '4'*work_dict[4] + '3'*work_dict[3] + '2'*work_dict[2] + '1'* work_dict[1] + '0'*work_dict[0] it could be replaced by a simple ''.join(str(i) * work_dict[i] for i in reversed(range(10))) Of course, ...

7

The question reads : You are given an array of numbers (not digits, but numbers: e.g. 9, 23, 184, 102, etc.) - you need to construct the largest number from it. For example: you get 21, 2, 10 - the largest number is 22110. (Emphasis mine) In a comment it was stated : The task basically was to decompose the numbers into digits and then rearrange ...

7

Your code contains a large amount of special-case handling, and as Peter Taylor already pointed out, it doesn't always produce correct results. Inputs with an even length that don't end with an a in particular, but also several odd-length inputs. Let's take a step back and look at the problem from a different angle: For a string s, there are s.Length + 1 '...

6

Review Well done, there isn't much I would change in this implementation. I would grant you the humble badge for putting yourself as LowLevel employee :) The lazy and null pattern are well implemented (except a small issue that could possibly introduce null -> see minor issues). The chain of responsibility looks to be by the book. You've even used a ...

6

Your code works correctly – as far as I can see – for strictly positive integers $a, b, c, d$. It fails if the integers are allowed to be zero or negative. For example: solve(-1, -1, -2, -3) returns False although $(-1, -1)$ can be transformed to $(-2, -3)$. solve(0, 0, 1, 1) fails with a “maximum recursion depth exceeded” because it calls itself ...

6

You didn't specify if you were allowed to use ecmascript-6 features or not but presumably you were, since you used let and arrow functions. As an interviewer, I would note that you used those features, yet you iterated over the array using a regular for loop instead of using for...of. That isn't necessarily a bad thing since it demonstrates that you know how ...

6

Little bit late but let me add another alternative solution, t3chb0t already took care of your code. You have extension methods for IComparable<T> and Char, this forces you to write your business logic in code instead of a (pseudo) high-level language which resembles your business rules. If you do not want to use a regular expression (I'd use it) then ...

6

... "too mathematical" and "lacking object oriented principles" These go hand in hand. An algorithm is what it is. Not that it is or is not too mathematical but that necessarily busy, complex, or obscure code is not wrapped in code that behaves (exposed) in subject/business terms. For example I think method and variable naming has a distinct algorithmic ...

6

Quite frankly it seems that you did not do what was asked from you. The text above assign gives you the requirements // Assign value val to interval [keyBegin, keyEnd). // Overwrite previous values in this interval. // Conforming to the C++ Standard Library conventions, the interval // includes keyBegin, but excludes keyEnd. // If !( ...

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