# Tag Info

15

Not duplicating any of @Peilonrayz's code review points ... Stop reading entire files into memory when you can process the file line by line in one pass, and stop creating huge lists in memory which are then iterated over exactly once. Both of these things creates a huge unnecessary memory pressure which can be avoided by looping and/or using generator ...

12

I think ask_file_name looks fine without using pathlib. The difference between the two comes down to LBYL vs EAFP. For the most part the difference between the two is style. Do you prefer using except FileNotFoundError or an if path.exists(). What I do find strange is ask_file_name follows a LBYL approach, but the code inside it follows an EAFP approach. ...

10

I like your solution quite a bit. It's clear, easy to read and I don't see any bugs. There are many ways to condense the replace calls as you mention, but I think you're at a point where such changes can easily have a disproportionate impact on readability. That's good--it means the code is already pretty optimal from that standpoint. For example, here's ...

8

You could save one .replace() call by replacing the last two with: .replace(/(^| ) +/g, "$1") which both removes leading spaces and collapses multiple consecutive spaces to one anywhere else in the string. However, I'm not 100% sure that you should, since it's not really clear which way is more efficient in practice, and your way seems more readable ... 7 fileExt="*.js" allFiles=$(find ./ -name $fileExt) This is a bug. The wildcard in$fileExt will be expanded by the shell, and cause a syntax error when the current directory has more than one matching file in it: $touch a.js b.js$ fileExt="*.js" $find ./ -name$fileExt find: paths must precede expression: `b.js' find: possible unquoted pattern after ...

6

DRY. Both branches do identical re.subs. Take them out: if chars is None: strip_left = re.compile(r'^\s*') strip_right = re.compile(r'\s*$') else: strip_left = re.compile(r'^[' + re.escape(chars) + r']*') strip_right = re.compile(r'[' + re.escape(chars) + r']*$') s = re.sub(strip_left, "", s) s = re.sub(strip_right, "", s) return s I ...

6

If you call regex_strip(s, ""), you will get: re.error: unterminated character set at position 0 because neither ^[] nor []$is a value regular expression. You could avoid this by using if not chars: instead of if chars == None:. There is no need to re.compile() your regular expressions; you aren't saving the compiled patterns anywhere for re-use. You ... 5 General Run shellcheck on this script - almost all variable expansions are unquoted, but need to be quoted. That will also highlight the non-portable echo -e (prefer printf instead) and a dodgy use of$(( where \$( ( would be safer. I recommend setting -u and -e shell options to help you catch more errors. Flexibility Instead of requiring users to change ...

3

Likely logic flaw In the first conditional: if (regex.test(pattern) && (arr.filter(s => s.includes(pattern)) != 0)) { Array.filter() "creates a new array"1 so the second conditional expression compares an array with zero. Those two should never be loosely equal so that conditional always evaluates to true. Perhaps you intended to check the ...

3

You could make the code better (and shorter) by using regex to split any delimiters that occur in the string, for example, in Hello.world, the string (list form) would then be like ['', 'Hello', ''] (after splitting the first word from delimiters) and then you can access the first word from index [1] (always). Like this, import re def first_word(s): ...

2

Requirements The requirements are somewhat imprecise because they rely on untold assumptions about the filenames we should expect. Before implementing anything, we should try to think about the different inputs we can have and how we should handle them. In our case, this could correspond to: what if the regexp does not match (no underscore or less than 3 ...

1

Your code actually doesn't work, because you use replaceAll. This means that your pattern will allow you to match ../../ and replace it with , resulting in a lost double-back. You can fix this two ways: You could change the replaceAll to replaceFirst You could change the pattern to exclude ../../ ([^/.]+/\\.\\./? works) You can then simplify your loop ...

1

I would like to hear how to make it more an one liner since I guess the code might be written with significantly less lines and still be readable. Excessively compressed (write-only): say sum m:g/\*<(\-?\d+\,?\d*/».trans: ',' => '.' given slurp Nice, readable, maintainable code if it can be assumed that later readers will know P6 or have enough ...

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