2
\$\begingroup\$

I am working on a script that recursively searches a directory to find files which matches a regex 5 or more times.

Using the script I can search for files which look like

https://github.com/nicolargo/glances/issues/1087
https://askubuntu.com/questions/758696/cannot-login-into-locked-ubuntu-14-04-session-unity
https://github.com/restic/restic/issues/14
https://www.huffpost.com/entry/our-shared-fears-and-how-_b_776720?guccounter=1
https://www.collaborativefund.com/blog/ideas-that-changed-my-life/
https://www.livescience.com/52882-charitable-acts-lead-to-bad-behavior.html

This has 5 or more lines that start with https://. If fewer than 5 lines start with https://, the file name will not be printed.

So far, what I came up with is:

from pathlib import Path
import re

for path in Path('/tmp/').rglob('*.adoc'):
    with open(path.resolve()) as file:
        count = 0
        for item in file:
            if re.match("^https://", item):
                count = count + 1
                if count > 4:
                    print(path.resolve())
                    break

The code has two loops and two conditions and it does not look right to me. How can I improve the code?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ What I find "interesting" is that the function doing a recursive search is not itself recursive. Here is a recent question that is somewhat similar with useful tips: Remove matching lines from all files. Besides, ask yourself is a regex is truly needed. It would be more economical to use item.startswith \$\endgroup\$
    – Kate
    Dec 6 '21 at 17:13
  • 1
    \$\begingroup\$ If you really want a regex, then you might as well go all out: if re.match('(^https://.*){5}','\n'.join(file.readlines()),re.MULTILINE): print(path.resolve()). That will probably be slower than startswith, but it may be easier to read. \$\endgroup\$
    – Teepeemm
    Dec 7 '21 at 1:03
3
\$\begingroup\$

This is so simple that there isn't much to improve.

In most cases you don't need a resolve() and should just omit this.

Use count += 1 instead of count = count + 1.

If you're always going to be checking for a prefix, use startswith instead of a regular expression.

Use path.open() instead of open(path).

Suggested

Not tested.

from pathlib import Path
from typing import Iterator


def files_with_pat(directory: Path, glob: str, prefix: str, minimum_matches: int = 5) -> Iterator[Path]:
    for path in directory.rglob(glob):
        count = 0
        with path.open() as file:
            for line in file:
                if line.startswith(prefix):
                    count += 1
                    if count >= minimum_matches:
                        yield path
                        break


def test() -> None:
    for path in files_with_pat(
        directory=Path('/tmp'),
        glob='*.adoc',
        prefix='https://',
    ):
        print(path)


if __name__ == '__main__':
    test()
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.