6
\$\begingroup\$

This is one of my first experiences with C#. I'm trying to learn and as can be seen, I need some obvious help. I know it's only an example but I'd like to start with best practices from the beginning. This was my first experience with public and private methods.

What can I do to improve my code and why is the change better? What about it makes it preferred or a best practice?

using System;

namespace CallMethodsWithinSameClass
{
    class Program
    {
        static void Main()
        {
            bool I_Has_Energy = true;
            OtherClass TestClass = new OtherClass();
            I_Has_Energy = TestClass.NoIDont;
            Console.WriteLine("Do I have energy? " + I_Has_Energy);
            Console.WriteLine("Getting a public method " + TestClass.GiveMeAString());
            Console.WriteLine("Grabbing the privates " + TestClass.GrabThemPrivates());
            Console.ReadKey();
        }
    }

    class OtherClass
    {
        public bool YesIDo = true;
        public bool NoIDont = false;    
        public string SimpleString = "This is only a test.";
        public string GiveMeAString()
        {
            return "Here's your sign, I mean string.";
        }
        private string KeepItToYourself()
        {
            return "Make sure you'll be gentle with them... *Wink*";
        }
        public string GrabThemPrivates()
        {
            return KeepItToYourself();
        }
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Well, humor is definitely a best practice ;) \$\endgroup\$ – Moshisho Mar 19 '17 at 9:06
  • \$\begingroup\$ It was the only way I could get something on the screen to start learning, and it worked. \$\endgroup\$ – IvenBach Mar 19 '17 at 18:16
5
\$\begingroup\$

I have a few tips for your code:

  1. Keep variable names consistent. Doing this makes it easier to remember (or guess) what you named them in the future. One good way of doing this is capitalizing all variable names the same way. Most programmers use what's known as camel case.

    Example:

    int camelCaseRules;
    

    This will help you A LOT in the future when coming up with coherent variable names, and since most programmers use camel case, it will help you in team projects.

  2. Another thing about variables is to make sure that you can understand it. E.g. don't name a variable NID (Standing for noIDont). When naming variables, you should make sure that you can get a basic understanding of what they do/are used for without digging in the code to figure it out.

  3. Comment, comment, and comment. Keeping up good commenting habits early on in programming will help you tremendously in the future. This will heavily increase your code readability and make it so that when you come back to the code in a week, you can understand why you did something the way you did it. Just putting in those two forward slashes makes a huge difference. Something to keep in mind when commenting your code however (which was brought to my attention by Hosch250), is to not comment on what the code does, but what it is used for. Basically, well written code should explain what it does without comments, and comments should explain what it is used for in the program.

    // Comment your code!
    
  4. Finally, only make this public if they have to be (e.g. your YesIDo variable in the OtherClass class. I would explain why, but I think this article does a much better job of doing so.

Also, a good YouTube video I would check out is here.

\$\endgroup\$
  • 6
    \$\begingroup\$ All good points except #3. If you have to write a comment so you know what your code does, then rewrite your code so your code tells you what it does. Writing a comment to understand your code == Fail. \$\endgroup\$ – Hosch250 Mar 19 '17 at 2:09
  • \$\begingroup\$ I've read up on public/private and am starting to understand it better. Your link really helped with my comprehension as to why. Is there a better manner in which I can use the Console.WriteLine? I have seen some string stuff with $ in front of it (msdn.microsoft.com/en-us/library/dn961160.aspx) but don't understand anything I read of it. I've seen also read about String.format (msdn.microsoft.com/en-us/library/…) and between each I'm not sure what's best to use. Does it depend, and if so can you provide examples/context? \$\endgroup\$ – IvenBach Mar 19 '17 at 3:06
  • 1
    \$\begingroup\$ @IvenBach Interpolated string it's simple way of avoiding ugly concatenation. Say you have taken the user's name and age and you saved those in 2 variables respectively string name and int age, say you want to print that you can either do Console.WriteLine("Hello " + name + ", your age is " + age + "."), that doesn't seems really nice, with interpolated string - Console.WriteLine($"Hello {name}, your age is {age}."). Here {name} and {age} are arguments they are not reviewed as simple text anymore but rather as value of variable or object. \$\endgroup\$ – Denis Mar 19 '17 at 3:15
  • 3
    \$\begingroup\$ @Cornchip - That's good advice, but it's easy to misinterpret it. Comments should explain the idea behind the code, not the code itself. Well written code will tell you what it does, but only comments can explain the business logic being coded (the why). Something like // Calculate the price can be replaced by well-written code. Something like /* Before we can display this to the user, we need to factor in all the appropriate shipping, taxes, and discounts, so they know the amount that will be charged to on their card */ is much better. \$\endgroup\$ – Bobson Mar 19 '17 at 4:40
  • 1
    \$\begingroup\$ @Bobson Wow, this makes a lot more sense to me than what my teacher and various YouTube videos have been telling me. Thanks! \$\endgroup\$ – Cornchip Mar 19 '17 at 12:50
4
\$\begingroup\$

A Few Things I Would Add:

  • Usage of readonly and const where possible. They make your code more readable and thread-safe. Basically, they add information and constraints on your variables. For instance, in your example YesIDo and NoIDont should be const. Now, if you had a variable like private timeInitialized it would make sense to initialize it once in the constructor with this.timeInitialized = DateTime.Now; and it can't be changed later on.
  • Class's name and role. I understand you named it only for the example, but it's something to think about when writing classes. If you follow SOLID principles there's the Single Resposibilty Priciple which says your class should have one responsibility only, which also makes it smaller, in your case it would mean something like: class Answers and class AccessModifiresExample

  • Instance names are usually camel case (as opposed to calling a static Class.Variable) so when I read your code I don't misinterpret TestClass.NoIDont as a static variable of TestClass.

  • Documentation. As in ///, is also a best practice. (everybody say it, but no one has the time to get to it ;)

\$\endgroup\$

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.