32

In the last three years that I have worked as developer, I have seen a lot of examples where people use a switch statement to set the path (both in back-end and front-end) for a URL. Below is an example of this:

Back-end example (C#):

public static string getHost(EnvironmentEnum environment){
    var path = String.Empty;
    switch (environment)
    {
        case EnvironmentEnum.dev:
            path = "http://localhost:55793/";
            break;
        case EnvironmentEnum.uat:
            path = "http://dev.yourpath.com/";
            break;
        case EnvironmentEnum.production:
            path = "http://yourpath.com/";
            break;
    }
    return path;
}

Front-end example (JavaScript):

(function () {
    if (window.location.host.indexOf("localhost") !== -1) {
        window.serviceUrl = "http://localhost:57939/";
    }
    else if (window.location.host.indexOf("qa") !== -1) {
        window.serviceUrl = "http://dev.yourpath.com/";
    }
    else {
        window.serviceUrl = "http://yourpath.com/";
    }
})();

It has been discussed whether it is a good or bad practice, and I think it is a bad practice, because we must avoid this kind of code and set a proper configuration. But to be honest I really don't know the proper answer and why is it not recommended and what is the correct way to implement this.

can someone explain it the pros and cons of the above practice?

Desolate Planet
  • 6,038
  • 3
  • 29
  • 38
gon250
  • 476
  • 4
  • 10
  • 13
    This line alone is not optimal. path = "http://yourpath.com/"; Configuration should be external to code. – paparazzo Jul 06 '15 at 14:38
  • 2
    From a pure code review perspective, a `Dictionary` is a much cleaner way of coding this in C#. See https://ideone.com/45g5xO. Or in JS use a good-old object, see http://jsfiddle.net/1ouhovqq/. – Danny Beckett Jul 06 '15 at 19:51
  • 4
    what happens if your company name changes to something that contains "qa"? – user253751 Jul 06 '15 at 20:29
  • Remember that if you use a config file it needs to be controlled in source code control.... Any you may have to edit the config file many times a day when you setup new test machines. I still think a config file is best, but you may wish to first look for a file named based Environment before looking at the detaul config file. – Ian Jul 06 '15 at 21:00
  • I'm wondering that noone mentioned that the c# example returns null (for a non-matched item) where even the js-example returns something senseful. Quit this comedy company before you start acting like they do. – ott-- Jul 06 '15 at 21:43
  • 1
    I don't think you should go around calling things bad practice if you cannot quantify why you think it is a bad practice – Roy Jul 07 '15 at 13:09
  • You might say it is bad practice because it makes an assumption, and this assumption is now hard-coded. However, I've done this before and I'm sure thousands of others have. It happens when we're busy thinking about other things. It's why refactoring and code reviews are so useful. – Daniel Hollinrake Jul 08 '15 at 08:09

5 Answers5

82

Code that works for you and is easy to maintain is by definition "good". You should never change things just for the sake of obeying someone's idea of "good practice" if that person cannot point out what the problem with your code is.

In this case, the most obvious problem is that resources are hard-coded into your application - even if they're selected dynamically, they're still hard-coded. This means that you cannot change these resources without recompiling/redeploying your application. With an external configuration file, you'd only have to change that file and restart/reload your application.

Whether or not that is a problem depends on what you do with it. In a Javascript framework that is automatically redistributed with every request anyway, it is no problem at all - the changed value will propagate to every user the next time they use the application. With an on-premises deployment in a compiled language in an inaccessible location it is a very big problem indeed. Reinstalling the application might take a long time, cost a lot of money or have to be done at night to preserve availability.

Whether or not hard-coded values are a problem depends on whether your situation is more like the first example or more like the second example.

Kilian Foth
  • 107,706
  • 45
  • 295
  • 310
  • 15
    I love your first paragraph. Sure, you follow it up with... pointing out what the problems are... but the idea that "this is bad because blog XYZ said so" is the cause of more bad code than it actually prevents. – corsiKa Jul 06 '15 at 16:24
  • I would disagree with hard coded resources being okay in automatically distributed files. If it's "hard coded" in the sense that the server somehow *injects* a value from configuration at runtime, that's not a problem, but hard coding it has many of the same problems as manually distributed: the server side will have to be redeployed to make the new file available to end users. It also lends itself far too well to ending up with the value stored in multiple places, complicating configuration and leading to "bugs" (that are essentially configuration issues), so it hurts your maintainability. – jpmc26 Jul 06 '15 at 17:47
  • 5
    Beginners should be given time-tested principles to live by, not simply *"if it works for you then it is OK"* relativism. I guess I am not wrong to say that hard-coding environment values is downright bad practice under any light. – Tulains Córdova Jul 06 '15 at 18:17
  • 3
    @jpmc26: You are of course assuming that deploying server side code is non-trivial, and also that there is some separate process through which configuration values can be updated with less overhead. Neither is necessarily true. Many shops have very little overhead in their deployment processes. On the other side, there actually are some shops where configuration changes have basically as much overhead as deploying changed code. (Validation, needing approval from a whole bunch of people, etc). The duplication concerns are definitely valid though. – Kevin Cathcart Jul 06 '15 at 19:31
  • @KevinCathcart Well, yes, but I generally assume prod to be under some lock and key, at least. Which means at least the overhead of going through the release process. Unless you have someone who would go dig into the code files and change them, which I kind of hope you wouldn't. Even if your reconfiguration process is just, "redeploy," when designing the system to use configuration, you can redeploy an existing, known to be working version instead of building a new one. – jpmc26 Jul 06 '15 at 21:05
  • One such example where updating config is as hard as redeploying was Microsoft/Visual Studio One Click deploy (this was 4 years ago, they may have fixed that.) – Frames Catherine White Jul 07 '15 at 00:54
  • 2
    With environment settings mixed in with your application code, you're one logic error—or unanticipated change in the execution environment—away from hitting test from production, or production from test, or some other unexpected and possibly catastrophic combination. Maintaining environment-specific properties separate from the code in C# is generally trivial. Why take an unnecessary risk? – John M Gant Jul 07 '15 at 15:47
  • 1
    @user61852 "I guess I am not wrong to say that hard-coding environment values is downright bad practice under any light." parses to "hard-coding environment values is always bad practice" If it is always bad practice, then it should always be possible to identify at least one problem that hard coding environment values will cause. – emory Jul 07 '15 at 18:33
14

You are absolutely right in thinking this is a bad practice. I've seen this in production code, and it always comes back to bite you.

What happens when you want to add another environment? Or change your development server? Or you need to fail over to a different location? You can't because your configuration is directly tied to code.

Configuration should be forced out of code and into the environment itself. It's a principle of a Twelve-Factor App (http://12factor.net/config), but it's a good practice for any application. You may find that environment variables aren't appropriate for your situation, in which case I'd suggest looking at storing that configuration in a database of configuration file alongside (but not checked in with) code.

mgw854
  • 1,818
  • 10
  • 11
  • If you don't track the configuration file, how do you know if the config file you have is valid for the version of software you just checked out of your VCS. (i.e. an untracked config file is not different to an untracked source file - you cannot build and deploy from VCS checkout without it) – mattnz Jul 07 '15 at 00:54
  • I don't disagree that an untracked configuration file is a difficult proposition. The way I've dealt with this before is two-fold: one, the binary for deployment also contains an XML Schema defining its configuration (so you can verify that a given config file will work). Second, the configuration files for each environment were stored in a document control system with different folders for each environment. Something similar could be done in Git with branches--version controlled, but discriminated from environment-less code. – mgw854 Jul 07 '15 at 01:29
5

For one, (as others have mentioned) this is a bad idea because you're tying implementation details into your code. This makes it difficult to change things.

As mentioned in this answer, if you want to add a new environment now you have to update your code everywhere, instead of just adding your program to a new environment.

There is another serious flaw with doing this in your Javascript code: You're exposing the internals of your company to potential attackers. Sure, you may be behind a firewall, but you still may have a disgruntled employee or someone who lets a virus in.

Bad news bears.

The best thing to do is to set your configuration from the environment (as in the previously linked answer, Twelve-Factor App has great advice on the topic). There are several ways to do this depending on your language. One of the easiest (usually) is to just set environment variables. Then you just change the variables depending on where you're running - whether that's a local dev box, qa, or production. Another option is storing the values in a .ini file or JSON. Yet another alternative would be storing your config values as actual code. Depending on what language or environment you're using this may or may not be a good idea.

But the ultimate goal is to let you take one code base, drop it on any machine that has the supported architecture/connectivity, and be able to run it without modifying the code in any way.

Wayne Werner
  • 2,340
  • 2
  • 23
  • 23
1

What if I want to run the backend on my own machine but not on port 55793, for example if I were running multiple versions at the same time to compare them? What if I want to run the application backend on one machine, but access it from another? What if I want to add a fourth environment? As others have pointed out, you have to recompile just to change the basic configuration.

The approach you have described might have worked in practice for your team so far, but it's pointlessly restrictive. A configuration system that allows parameters like this path to be set arbitrarily in a central configuration file is far more flexible than one that only provides fixed options, and what advantage do you gain with the switch statement approach? None!

PeterAllenWebb
  • 933
  • 8
  • 12
0

It's a BAD PRACTICE.

A basic principle of software design: Don't hard-code configuration values inside your programs. This is especially true for anything that has a reasonable chance of changing in the future.

The program code that you develop should be the same code that goes into any environment such as QA testing, UAT, and production. If somebody needs to change the configuration later because the environment has changed, or they need to use it in a new environment, fine. But they should never have to touch your program code to do so. And you should not have different versions of code for each environment. If your program has changed since it was tested, then you haven't tested that version. Ask any software engineer, any software quality assurance professional, any I.T. project management professional, any I.T. auditor.

Someone could move testing to another server. Someone might decide the also want a user training environment, or an environment for sales demonstrations. They shouldn't have to go to a programmer for administrative configuration.

Software should be flexible and robust enough to handle unexpected situations, within reason.

Furthermore, software should not be written simply however seems easiest for you at the moment. The cost of software development is small compared to the cost of software maintenance over its lifetime. And let's say a year from now, you might not be the person working on that code. You should be thinking of what you're leaving for the next poor fool who has to pick up whatever mess you you may have left behind, perhaps years after you've gone on to greener pastures. Or you may be the one who picks up the code years later, not believing what you did back then.

Code things properly, as best you can. It's less likely to become a problem later.

WarrenT
  • 601
  • 1
  • 4
  • 7