3
\$\begingroup\$

Introduction

1) Problem description:

Suppose you are managing files. You are given a text file, where in the first line are storage's volume of the server and the amount of users in total that is in the text file. Each line from the second to the end shows volume of individual user's storage. Based on the server's volume limit you need to find the maximum amount of users that can be fit in the database and the maximum volume among them.

2) Sample.txt

100 4
80
30
50
40

3) Sample output: 2 50

4) Sample output's explanation: The maximum amount of users that can be fit in the database is 2 (more than 2 will be more than 100 in total). All possible pairs, which meet volume's requirements, are 30 40 30 50 50 40 (each makes less than 100). The maximum number among these pairs is 50.


Actual problem

1) db.txt

9691 1894
89
24
33
11
...

2) My solution:

fName = open('../root_files/26_7791633.txt')
param = fName.readline().split()
data = [int(el) for el in fName.readlines()]
fName.close()

TARGET = int(param[0])

data.sort()


target_disposable = TARGET
max_num = 0
quantity = 0

# finding maximum amount
while target_disposable - data[quantity] >= 0:
    target_disposable -= data[quantity]
    quantity += 1

new_list = data[0:quantity]

element_in_the_end = new_list.pop()
remainder = TARGET - sum(new_list)

# finding the very maximum number based on quantity
for i in reversed(range(remainder + 1)):
    if i > element_in_the_end:
        max_num = i
        break

print(quantity, max_num)

3) Output:

34 601
Solution accepted
Runtime: 55ms

4) My question: Is there a way to make this code more concise, maybe even improving its runtime? Thank you in advance.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ (The "Solution accepted" output is funny - I'd expect the first number to be smaller than the second. And for the Sample, I get 2 70.) \$\endgroup\$
    – greybeard
    Dec 12 '20 at 17:06
  • \$\begingroup\$ @greybeard Yes, looks like a poorly-veiled attempt to trick us into fixing their non-working code. \$\endgroup\$ Dec 13 '20 at 5:43
5
\$\begingroup\$

Some suggestions:

  1. foo[:bar] is syntactic sugar for foo[0:bar].

  2. The idiomatic way to ensure that a file is closed when the code is finished with it is to use a context manager:

    with open(…) as fName:
        [any code which reads from fName]
    
  3. data is a literally meaningless name: anything stored in a variable is data. In this case you might want to use a name like user_counts or users.

  4. The return value of open is not a file name, but rather a file descriptor.

  5. Unless this is a one-off piece of code I would wrap the different bits (the input-handling code (reading the file), the central algorithm and the output code) into functions.

  6. By returning the result rather than printing it this code could be reused by other code.

  7. Since the code naturally only takes a single input stream, I would read the values from standard input (per the Unix philosophy). That way the code can be called with any file trivially, as in ./max_sum_inputs.py < ../root_files/26_7791633.txt

  8. A linter like flake8 or pylint will tell you about other non-idiomatic code, such as how variable names should all be snake_case.

\$\endgroup\$
1
1
\$\begingroup\$
  • Document, in your code, what it is to accomplish. Using the means the language provides, if any.

  • Picking up naming from the problem statement (or product description or requirements specification in professional programming) facilitates following what is what. (Especially for a tutor/customer who is familiar with "the original naming".)
    (I understood # finding maximum amount right away, and not for its (marginal) obviousness.
    By the same token, the next comment might read # finding the maximum volume. (If it was to include based on quantity, it better preceded "the slice" using quantity. Which wouldn't be a bad placement, anyway.)

\$\endgroup\$
1
  • 1
    \$\begingroup\$ l0b0 demonstrated commendable reticence in his answer - trying to follow. \$\endgroup\$
    – greybeard
    Dec 12 '20 at 9:51

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.