40

While writing a library for a large project I'm working on at work, an issue came up which required a token to be sent to an email address, and then passed back into the code where it can then be used for further use.

My colleague says to just read from STDIN (using Python: code = input("Enter code: ")) and then have a user pass it in, however to me this seems like bad practice as the library might (in this case definitely will) be used in a background task on a server.

I was wondering whether or not this was considered an anti-pattern or not.

Peter Mortensen
  • 1,050
  • 2
  • 12
  • 14
Paradoxis
  • 1,351
  • 1
  • 11
  • 10
  • 45
    Not everything bad is an "anti-pattern", though this is certainly bad. – Phoshi Feb 24 '17 at 15:19
  • 4
    "pattern" means something that programmers frequently do. It's only an anti-pattern if it's both (A) a bad idea, and (B) something that you see developers doing all the time. – Solomon Slow Feb 24 '17 at 18:47
  • 21
    This is far too dumb to be an anti-pattern. An anti-pattern is something that seems natural and sensible but turns out to be bad when you dig into it. What you're describing here is just obviously gruesome. – Evan Harper Feb 24 '17 at 22:35
  • I am not seeing the point of any of the answers. The point of the user token exercise must be to prove that the user's email address works. If that was not the point, then it would be far simpler to simply store the token. – emory Feb 25 '17 at 00:32
  • 2
    This is pretty awful. If passing a token like that is absolutely required, you might make a separate executable using the library and pass your token to its stdin. But to hijack the stdin of the calling executable, that's a no-no. – GrandmasterB Feb 25 '17 at 04:55

5 Answers5

78

As a general guideline, libraries should be totally disconnected from the environment. That means that they shouldn't perform operations on standard streams, on specific files, or have any expectation about the environment or the context they are used.

Of course, there are exceptions to this rule, but there must be a very good reason for it. In the case of using stdin, i can't find any reason (unless your library actually provides routines for reading from stdin, like std::cin from C++). Also, taking the I/O streams from a parameter rather than having them hardcoded adds so much flexibility that it's not worth not doing it.

Paul92
  • 2,571
  • 15
  • 17
  • 36
    The exception is a library that has a specific goal of interacting with an environment. Even then, details of the environment should be abstracted away. For example, a graphics driver has to communicate with the PCIe bus, but the bus ID should be provided via configuration. –  Feb 24 '17 at 15:44
  • This is the kind of exception I thought: my idea was closer to *ncurses* - generally, a text-environment user interface library. If its purpose is to read user input and provide user output, that's a good reason. – SF. Feb 24 '17 at 20:38
  • 5
    @SF. Even a library like *ncurses* should take a pair of file descriptors as arguments rather than hardcoding the use of 0 and 1. You might want to write a program where stdin and stdout can be redirected and instead you want to open `/dev/tty` in order to communicate with the user. The program might even be started without a terminal and open its own terminal using `xterm -S`. – kasperd Feb 24 '17 at 23:52
  • 3
    @kasperd: The best approach is providing sensible defaults and ability to override them. – SF. Feb 25 '17 at 01:02
  • @SF. One has to be careful not to use global variables for such overrides though. There could be usage cases where one program would want to use the same library twice. For example in the case of *ncurses* there might be a desire to show a local and a remote UI simultaneously from a single invocation of the program. There would be only one instance of the library in the address space of the program, but the data structures used by the library would have to be replicated. In the case of *ncurses* it would be a rare scenario, for other libraries it can be much more common. – kasperd Feb 25 '17 at 17:09
  • @kasperd: agreed, but that's implementation details. My point is that if you demand every least configuration detail on instantiation/initialization, one is usually better writing their own solution from scratch instead of using the library. Trivial use case should be trivial to implement; complexity growth proportional with complexity of use case. Making many such things as open file descriptor of stdin on initialization obligatory complicates implementation to a degree that makes use of library inferior to custom own solution. – SF. Feb 25 '17 at 18:12
  • 1
    In this particular case, I see no reason to demand a *stream* as input. Why not just accept the *token* as the parameter? – jpmc26 Feb 25 '17 at 20:05
16

I would consider this not necessarily an anti-pattern, just a poorly designed library. It should be trivial to ask for a string as a method parameter, where the input could be passed directly in.

If that doesn't fit this usage, then a method parameter can be a stream, with STDIN passed into the method.

If that doesn't fit this usage, then the library is not flexible enough.

Bryan Boettcher
  • 2,754
  • 4
  • 21
  • 31
4

Maybe consider having the ability in your library to set a callback to a user-provided function that will read input from wherever, and then return the appropriate value back to whatever part of the library is using that function.

FrustratedWithFormsDesigner
  • 46,105
  • 7
  • 126
  • 176
1

If it does read from stdin, it means it would like to take program-level ownership of stdin. It likely is not compatible with any other library that reads from stdin, less specific protocol for how they share use. In at least my own personal glossary, this would make the library a framework, which is an expensive tradeoff.

But in this case, the library should probably just take an input file descriptor.

djechlin
  • 2,212
  • 1
  • 15
  • 26
0

The answer by @Paul92 is good general discussion, but I would like to offer a possible clean(ish) solution to this:

A a library, this code needs to be adaptable to any runtime environment, so you can't really ask STDIN for some crucial bit of data. For one, users of your library might not have stdin available for a number of reasons. Instead you might want to use some form of strategy pattern to customise how the token is to be retrieved.

In Python, probably the best option is to pass in the token fetching strategy as a function parameter. Something like that:

def stdin_prompt():
    return input("Enter code: ")

def my_library_function(arg1, arg2, ... argn, token_provider = stdin_prompt):
    ...
    token = token_provider()
    ...
    return stuff

# somewhere in the user code
stuff = my_library_function(a1, a2, ... an, lambda: "123456")

Think of it like this. The token that you require, is an argument to the library function. Since the value for the token might not be statically known at the call site, you can not really ask for the value as an argument. Instead, the caller has to provide a function that is going to be responsible for providing the token when called.

All the responsibility for providing the exact mechanics of the token are now externalised from the library function. The consumer of the function is now responsible for acquiring the token by whatever means are available at runtime. It may ask STDIN, but it might also act as a mail gateway, wait for the message to pop in to the inbox, read it, extract the token and completely automate the process. It might be a GUI dialog or a web based form. Anything really -- all the options are now in the hands of the library consumer.

Roland Tepp
  • 179
  • 2
  • 9