The use of a large number of hard-coded lines in the code

Question

When I look at my code and I write things like ..

if (role == "Customer") { bCustomer = true; } else if (role == "Branch") { bIsBranch = true; } 

or

 foreach(DataRow as row in myDataSet.Tables[0].Rows) { row["someField"]=somefield.Tostring() } 

Do you guys do this? When is it ok to do, and when should you not? What if one of them is better for writing, if any?

Thanks for the comments: I think I should add that if (for this example) I only use this role comparison once? Is there still an idea to create a completely new class? Also, should I have 1 class called "constants", these are several classes that contain certain constants, for example, a class of "roles"?

Answer 1

Not. Do not use magic lines . Instead, create a static class with constants or an enumeration if you can.

For example:

 public static class Roles { public const string Customer = "Customer"; public const string Branch = "Branch"; } 

Using:

 if (role == Roles.Customer) { } else if (role == Roles.Branch) { } 

Here is a good discussion on various solutions .

Answer 2

It is always better to declare hard-coded strings separately as constants, rather than declare a new line each time. It keeps the code clean and reduces errors caused by input errors.

Relative to or should not be done is entirely script dependent.

Answer 3

I believe that the best approach is to use the application settings , which means that you no longer need to recompile your code if the "Client" or the "Branch" Values ​​change. Magical values ​​are obviously bad, and that would be a good first step / option to get away from them. In addition, it stores your values ​​in one place, and I also believe that you can reload settings at runtime without restarting the application (although I have not tried this myself).

eg:.

 if (role == Properties.Settings.Default.CustomerRole) { bCustomer = true; } else if (role == Properties.Settings.Default.BranchRole) { bIsBranch = true; } 

Answer 4

I would make a static Roles class:

 public sealed class Roles { public const string BRANCH = "Branch"; public const string CUSTOMER = "Customer"; public static bool IsCustomer(string role) { return role == CUSTOMER; } } 

Then in your code:

 bCustomer = Roles.IsCustomer(role); 

---

As an alternative, this requires a bit more tweaking, but RoleProvder (depending on the network or not) provides many good methods.

Answer 5

Well, in my opinion, it is up to you, and it depends on your application design. I confidently look at this from the positive side - if the application works the way it should work, everything is fine. IMHO

Answer 6

Polymorphism is one thing, but using hardcoded strings along your code is not very good. It is better to define a variable containing a string and use this variable along the code. In this case, if you need to change something (believe me, you will), you can just change the value of this variable and do it (fewer errors too!)

Answer 7

For ease of maintenance, you should formalize string comparators when possible, either as named constants or as an enumeration. The benefit to the programmer is that you can localize the changes. Even with the refactoring tool, finding all the places where the string is used can be tedious and error prone. You can have only one place where you do this comparison today, but you or the future maintainer can extend this to other parts of the code. In addition, the class itself can grow and must be broken down. These things tend to creep along the program over time.

I would simply declare these lines as constants close to where they are used, but together. Do not worry about a new abstraction, like Roles, until you know that you need it. If the number of roles you need to compare is growing or is required outside this class, then you can create a class of roles or roles depending on the complexity of the comparison.

In addition, using constants, you signal the intended use of the compiler, so you get some minor memory management benefits, which, given that your comparison is in a loop, are usually good practice.