Hot answers tagged

8

Welcome to Code Review! I'll add to the other answer from Reinderien. PEP-8 In python, it is common (and recommended) to follow the PEP-8 style guide for writing clean, maintainable and consistent code. Functions and variables should be named in a lower_snake_case, classes as UpperCamelCase, and constants as UPPER_SNAKE_CASE. No unnecessary whitespaces are ...


7

Path? Surely path is not a single path, since you loop through it. So at the least, this is poorly-named and should be paths. The variable you iterate through it, fileName, should be file_name; similarly for your other function and variable names. Line iteration Rather than calling readlines(), simply iterate over file itself, which will have the same effect....


6

Type hinting Instead of having helpdocs declare the types of function parameters, why not go with type hinting? Complexity Your code currently has too many moving parts. You define 2 different functions to parse the data, and they both need to be called in chain. This should be done by a single parsing function. Let the parser get data text, then the parser ...


5

I can see three small improvement areas: Use ctor instead of object initializer Instead of this: new DataColumn(v.Key) { DataType = v.Value.GetType() } Use this: new DataColumn(v.Key, v.Value.GetType()) Object initializer will run after ctor. If ctor accepts that parameter then prefer to provide it in the ctor. Use early exist instead of guard expression ...


5

In addition to the points raised in the other answers: Extraneous import The first line is import sys, but I don't see sys used anywhere; this can go. Explicit naming It's generally better to use explicit names. Thus, rather than just seq (which might be short for sequence or sequences, for example) spell it out. In this case, it looks like you should use ...


4

Rather than declaring the variables with let (you should always prefer const) and concatenating and reassigning, consider creating arrays of numbers and dates instead, eg: [2, 3] and ["2020-09-15", "2020-09-16"] Then after the loop is done, join all elements by newlines. For the regular expression, rather than split, I think match would ...


3

Just some minor things in your code. Is a bit strange that you have a function call free_url and you dont have one called init_url where you alloc and do the memset. My suggestion is that you have another function for that and you move your memset of the function int parse_url(char *start, struct url *url) { char *end, *delim; memset(url, 0, sizeof(*...


3

Edge cases In the real world, input isn't always well-formed. What happens if there is a typo in the data? For example: const data = [ "(2) 2020-09-15", "(3] 2020-09-16" ]; This will lead to undefined appearing in the output for the dates. In other cases/frameworks/languages an exception might be thrown that could crash your script/...


3

ExpressionParser is too general, you might need to renamed it to be specific such as MathExpressionParser. ExpressionParser should be abstract class along with Variables property. Computer in ExpressionParser can be declared globally, and used across the class. in the Compute method, you're not validating the string, you should add some validations such ...


3

Nice, type hints! One thing I might start with, after working with a strict mypy configuration, is to make the type declarations stricter, and then enforce that strictness. For example, a type of list is equivalent to list[Any]. In the case of the children parameter you know more than that: it's list[str]. mypy has an option to disallow "any" ...


2

Here's an implementation whose major endeavour, when compared with your implementation as well as that of the accepted answer, is separation of parsing and execution. It's unclear whether this is important for you, but it's generally good design, and is likely faster to re-execute once parsed: import re from numbers import Real from random import randint, ...


2

I don’t have enough reputation to comment, so I must say this in an answer. It looks clunky to use while x == 4: and then do x = 3 whenever you want to break out of the loop. It looks better to do while True: and when you want to break out of the loop do break Cheers!


1

Redundant Code pagetext = "" pagetext = (page.text) The first line assigns an empty string to pagetext. The second line ignores the contents already in pagetext and assigns a different value to the variable. Why bother with the first statement? It simply makes the code longer, slower, and harder to understand. Why bother with the (...) around ...


1

I would use something like this: const data = [ "(2) 2020-09-15", "(3) 2020-09-16" ]; let pairs = []; const regex = /^\((\d+)\)\s+(\d{4}-\d{2}-\d{2})$/; for (let datum of data) { // Idea from the previous answer: let pair ...


1

For such a simple parsing job, I'd use a regular expression and re.findall()` to split the string into a list of tokens. It's just one line instead of over 20 lines. The tokens are an element (an upper case letter possibly with a lower case letter), a repeat count (1 or more digits), or a bracket. The "{}[]" matches any of the brackets. tokens ...


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