Is this considered best coding practice?

Question

I have seen a video tutorial in C# and I followed it to learn C#. However, I am not sure if the author does follow best practice or not. What I asked myself about is why didn't he create a method to print the speed rather than Console.WritLine that in two different methods As follow:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace understandingOO
{
    class Auto
   {
    public string Make;
    public string Model;
    public int Year;
    public string color;
    public int Miles;
    public int Speed;


    public void Accelerate()
    {
        Speed++;
        Console.WriteLine("Current Speed: " + Speed);

    }

    public void Deaccelerate()
    {
        Speed--;
        Console.WriteLine("Current Speed: " + Speed);
    }
 }
}


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace understandingOO
{
    class Program
  {
    static void Main(string[] args)
    {
        Auto myCar = new Auto();
        myCar.Make = "Toyota";
        myCar.Miles = 5000;
        myCar.Model = "Cutlas..";
        myCar.Speed = 0;
        myCar.Year = 2011;

        myCar.Accelerate();
        myCar.Accelerate();
        myCar.Accelerate();

        myCar.Deaccelerate();

        Console.ReadLine();

    }
}
}

Answer 1

You are right.. He should have made a method PrintCurrentSpeed(). This is a basic tutorial and he probably wanted to keep it simple.

Answer 2

If you are worried about "Best Practice", he should have made the variables private and created methods to access them, created a Constructor and Destructor for the class, commented the code, the magic numbers 5000 and 2011 and strings should be defined constants. I am sure others can add to this.

However all that would get in the way of the purpose of the program, (an introductory tutorial). In fact, if he had written this program to "Best Practice" standards, it would no longer fulfill it's purpose, making it defective even though the program outputs are the same.

Often best practice is choosing, for valid reasons, not to do "Best Practice", which is why "Best Practice" is so rarely practiced, and even more rarely best practice.

Answer 3

My guess would be that if the intention was to teach a beginner that principles like DRY may not be the focus. Thus, this isn't perfect as to try to get there may be confusing to people that may have trouble understanding methods.

Answer 4

Multiple things wrong with the given code:

1. He should've used properties instead of fields for public access.

2. Speed should be a property with a { get; private set; } signature. Since methods Accelerate and Decelerate are used to control the speed, it makes no sense to be able to change the speed "manually" by editing the field value. A lot of .NET classes work this way in that they prohibit you from messing with internal values directly.

3. I disagree with one of the above posts which states that various values should've been defined as constants. It makes no sense to do that since it's arbitrary data. What the programmer should've done is creating a constructor for the class. Having a constructor atomizes creation operation and in a way guarantees that the object is now ready to be used. By manually setting the values from other classes, you never know when the class is finally ready to be utilized.

Answer 5

I'm just going to guess, that since it is a simple example, he used writing to console as the simplest possible way of showing what's going on when methods are called.

In 'real' applications, you usually don't print/log changes in object variables. You usually check their values only when needed. In this example you woud not have a PrintSpeed() method. Rather, you'd have GetSpeed(), which could be used by any other object, to either print the speed, or use for some calculations.

Answer 6

Send the correction to author.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace understandingOO
{
    class Auto
   {
    public string Make;
    public string Model;
    public int Year;
    public string color;
    public int Miles;
    public int Speed;


    public void Accelerate()
    {
        Speed++;
        //Accelerate shouldn't write to the console
        //Console.WriteLine("Current Speed: " + Speed);

    }

    public void Deaccelerate()
    {
        Speed--;
        //Deaccelerate shouldn't write to the console
        //Console.WriteLine("Current Speed: " + Speed);
    }
 }
}


using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace understandingOO
{
    class Program
  {
    static void Main(string[] args)
    {
        Auto myCar = new Auto();
        myCar.Make = "Toyota";
        myCar.Miles = 5000;
        myCar.Model = "Cutlas..";
        myCar.Speed = 0;
        myCar.Year = 2011;

        myCar.Accelerate();
        myCar.Accelerate();
        myCar.Accelerate();

        myCar.Deaccelerate();

        //Show car speed
        Console.WriteLine("Current Speed: " + myCar.Speed);


        Console.ReadLine();

    }
}
}