19
\$\begingroup\$

I have been practising Python Practice questions and came across this question:

Write a Python program to add 'ing' at the end of a given string (length should be at least 3).

If the given string already ends with 'ing' then add 'ly' instead.

If the string length of the given string is less than 3, leave it unchanged. Sample String : 'abc'

Expected Result : 'abcing'

Sample String : 'string'

Expected Result : 'stringly'

Here's my code:

string = input()
if len(string) < 3:
  print(string)
elif string[-3:] == 'ing':
  print(string + 'ly')
else:
  print(string + 'ing')

However, something about this code seems wrong: though it works, fine I feel it smells bad. I can't pinpoint why.

\$\endgroup\$
29
\$\begingroup\$

1. Python strings have an .endswith() method. Use it.

While string[-3:] == 'ing' isn't particular hard to read, string.endswith('ing') is even clearer.

Potentially, depending on how it's implemented, .endswith() could even be slightly more efficient, since it doesn't have to copy the last three characters of the string. However, always keep Knuth's maxim in mind. The entire operation of testing whether a string ends with "ing" is so trivial that any performance difference between different ways of doing it will surely be negligible compared to e.g. the time consumed by reading and printing the string. Thus, given two ways to do it, you should choose the most readable and idiomatic way even if it wasn't the most efficient.

2. Separate I/O from computation.

It's common for beginning programmers to litter their code with input() and print() calls in the middle of program logic, simply because those are the only ways of receiving and returning data they're familiar with. But in practice, that's almost always a mistake.(*)

Your main task is to modify a string. Reading the string from standard input and printing the result to standard output are merely auxiliary tasks to that (and may or may not even be strictly necessary, unless the exercise explicitly specifies that particular method of input and output). You shouldn't unnecessarily intermix those auxiliary tasks into your solution for the main task.

The simplest way to do that would be to just pull the print() calls out of the program logic, e.g. like this:

string = input()

if string.endswith('ing'):
  string += 'ly'
elif len(string) >= 3:
  string += 'ing'

print(string)

This is OK if you expect to only ever use this code for processing data read from one file (or pipe) and written to another. You're still doing the input, processing and output in one linear program, but at least the processing logic is now grouped together in a single block of code that you can extract from the script if you want to reuse it, and the I/O code is clearly separated from it, making it easy to change to another I/O method later if you need to.

Alternatively, you could go a step further and wrap the program logic in a function that takes the input string as a parameter and returns the output. And, while you're at it, you can use the if __name__ == '__main__' idiom to make your I/O code only run when your script is invoked directly:

def inglify(string):
  if len(string) < 3:
    return string
  elif string.endswith('ing'):
    return string + 'ly'
  else:
    return string + 'ing'

if __name__ == '__main__':
  string = input()
  print(inglify(string))

The advantage is that now your script file is also usable as a module that you can import into another script to gain access to any functions defined in it without executing the I/O part of the code.


*) So, when is it appropriate to include print() calls directly in your code, then? Off the top of my head, I can think of three or four cases (with some partial overlap between them):

  1. When I/O is the main purpose of your code, with everything not directly relevant to this purpose already split off into reusable functions. For example, print(inglify(input())) really does nothing but read data, call a function and print the result. Obviously, using print() there is not only appropriate but necessary.

  2. When your code is interactive, i.e. when it prints something to the console, expects the user to reply, then prints something in response, etc. The REPL that you can start by running python with no script file argument is one example of such a program. In this case the input and output are intrinsically part of your program logic and cannot be easily separated from it. That said, you should still try to keep such interactive loops as simple as possible, and split off any parts of your code that don't directly involve interactive I/O into reusable functions or modules.

  3. When you're doing debug printing, i.e. when what you're printing isn't the actual intended output of your program, but merely some metadata to help in tracing its execution. This can sometimes be a very useful ways to find out what your code is actually doing, and it's the one case where putting print() or other I/O calls even deep inside your program logic can be acceptable. Of course, if you're leaving such debug logging in your finished code, you should probably only enable it if requested by the user. (For example, many command-line tools will accept a --verbose or --debug switch to turn on such extra output.) Often it's best to use an actual logging framework designed for this purpose.

I would also personally include an exception for "simple data processing" scripts that involve nothing more than reading in some data, processing it and printing it out, much like the first rewritten version of your code that I suggested above.

Such scripts are characterized by the fact that they only perform a single task, that this task involves reading, processing and writing data, and that their control flow is simple and mostly linear. That is, such a script might consist of a loop that reads the input, another loop or two to process it, and then a loop to print out the results. Sometimes it might even have a loop that e.g. reads a line of input, processes it, and prints out the result before reading the next line, etc. But what it definitely shouldn't have is print (or, worse yet, input) calls buried deep within conditional code, or in functions whose main purpose isn't obviously to perform I/O.

While such scripts can be refactored to separate the I/O code from the rest of the data processing, often that can actually make the code harder to read, since it breaks up the simple step-by-step program flow into a more complex sequence of function calls. Basically, if your code is naturally structured like a cooking recipe, with nothing but simple sequential steps, it's sometimes better to just keep it like that and not complicate it by needless refactoring.

\$\endgroup\$
3
  • 2
    \$\begingroup\$ This is incredibly well written and explanatory thank you:) \$\endgroup\$ Nov 18 '20 at 18:58
  • 1
    \$\begingroup\$ I would argue that while modularization and reusability is generally important, forcing those principles onto a 7 LOC script is over-engineering. The advice about .endswith() is definitely valid though \$\endgroup\$
    – kangalioo
    Nov 19 '20 at 14:21
  • 5
    \$\begingroup\$ @kangalioo: The point of exercises like this is to develop coding skills that can be applied to actual real-world tasks. Arguably, following good coding style for such exercises is more important than it usually would be. Normally, "this is throw-away prototype code, I'm never going to reuse it" can be a valid argument for cutting corners. When training your programming skills, however, you should always write the code as if you were going to reuse and extend it later, even if you know you really won't. \$\endgroup\$ Nov 19 '20 at 14:27
10
\$\begingroup\$

The only thing somewhat bothering me is the short indentation :-P (standard is four spaces)

And endswith is better in general as it avoids the cost of the slice copy, doesn't require stating the length, and also works for empty test-suffixes:

>>> for s in 'o', 'a', '':
        print('foo'.endswith(s),
              'foo'[-len(s):] == s)

True True
False False
True False         <= Note the difference!

Here's how I might do it:

string = input()
if len(string) >= 3:
    string += 'ly' if string.endswith('ing') else 'ing'
print(string)
\$\endgroup\$
6
  • \$\begingroup\$ My small quibble with this would be that you're overwriting the original string. \$\endgroup\$
    – Ben
    Nov 18 '20 at 16:42
  • 2
    \$\begingroup\$ @Ben Yes, as instructed. \$\endgroup\$ Nov 18 '20 at 16:43
  • \$\begingroup\$ Very true; the whole printing thing is throwing me off. I'll leave my original comment there as your response is valuable. \$\endgroup\$
    – Ben
    Nov 18 '20 at 16:45
  • \$\begingroup\$ I also just noticed that we need to make changes in the string itself thanks :) \$\endgroup\$ Nov 18 '20 at 17:25
  • \$\begingroup\$ @FoundABetterName Well, we can't really change the string itself, as it's immutable. I'd say closest thing we can do is change the variable (which is what I do). And of course if you really just input and print, it doesn't really matter. \$\endgroup\$ Nov 18 '20 at 17:36
5
\$\begingroup\$

I think the code looks fine, except you could use the library function to check it ends-with "ing":

string = input()
if len(string) < 3:
  print(string)
elif string.endswith('ing'):
  print(string + 'ly')
else:
  print(string + 'ing')
\$\endgroup\$
3
\$\begingroup\$

There's not many other ways to do it.

You could replace the if-elif-else statement with an if-elif-else expression. Is that more readable or less?

You could use the if statements to pick the appropriate suffix and have just on print at the end.

In the print statement, you could use an f-string instead of string concatenation

string = input()
suffix = '' if len(string) < 3 else 'ly' if string.endswith('ing') else 'ing'
print(f"{string}{suffix}")
\$\endgroup\$
2
  • 2
    \$\begingroup\$ I find this if-elif-else expression too long to grok easily. @Heap Overflow's answer, using an if len>=3 statement controlling a string += if-else expression, looks like a good balance to me. And has separation of logic from I/O. \$\endgroup\$ Nov 18 '20 at 20:50
  • 1
    \$\begingroup\$ @PeterCordes, I agree. As I implied in my answer the if expression might not be as readable. \$\endgroup\$
    – RootTwo
    Nov 19 '20 at 0:07

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.