1
\$\begingroup\$

My challenge is to transfer examples from textbooks or videos into something I am coding on my own.

Environment

Visual Studio 2022
Net 7.0
ConsoleApp

My goal

My idea is to have a solution to provide messages to a console. Like "press enter to continue ..." or "you are leaving the app ...". And some functions like "clear screen" or "resize screen". Thus, nothing really smart or special. In the long run the view SettingView shall display some setting information on the console.

My goal is to get used to the SOLID principles. So far, I was putting everything in just a few classes (the less the better) and I was hardly using any interfaces. It is kind of challanging to get rid of these old coding habits.

To reach my goal I did the following

  • Splitting up the huge class and interface into smaller ones. Each of the should perform simple tasks. e.g. classes implementing IMessaging just have a Show() method to display some content on the console.
  • Using interfaces to make it easy to exchange classes. e.g. SettingView does have the property StartMessage of type IMessaging. To alter the output on the console I just need to replace instancing StartingView with StartingApp.
  • Implementing dependency injection. e.g. SettingView and ISettingView. Instead of implementing all the interfaces like IMessaging I have created properties which contain all the logic, like StartMessage of type IMessaging. I just wanted to have one method in ISettingView and this was Run. That should be the starting point when GetService<ISettingView> is called.
  • Making the code expandable. This I tested by not just having one class StartingView but also to have the class StartingApp. In this second class I could do different stuff, but I would not have to change any code in the main logic of SettingView.
  • To follow the MVVM pattern I have created SettingViewModel but I am not sure what to put there. The ViewModel should host the logic of the View. And so far I am just working on the View itself. I suppose all the information that will be displayed in ShowContent() should be provided by the ViewModel.

Source Code

The project is available on github.

Program.cs

using BasicCodingConsole.Views.MainView;
using BasicCodingConsole.Views.SettingView;
using BasicCodingLibrary.Models;
using BasicCodingLibrary.ViewModels;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Serilog;

#region ***** Configuration *****
var builder = new ConfigurationBuilder();
builder.SetBasePath(Directory.GetCurrentDirectory())
    .AddJsonFile("appsettings.json", false, true)
    .AddJsonFile($"appsettings.{Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT") ?? "Production"}.json", true)
    .AddEnvironmentVariables()
    .AddCommandLine(args);

Log.Logger = new LoggerConfiguration()
    .ReadFrom.Configuration(builder.Build())
    .Enrich.FromLogContext()
    //.WriteTo.Console()  // activate to send the logging to the console
    .WriteTo.File("LogFiles/apploggings.txt") // activate to send the logging to a file
    .CreateLogger();
var host = Host.CreateDefaultBuilder()
    .ConfigureServices((context, services) =>
    {
        services.AddLogging();
        services.Configure<UserInformation>(builder.Build().GetSection("UserInformation"));
        services.Configure<ApplicationInformation>(builder.Build().GetSection("ApplicationInformation"));
        services.AddTransient<IMainView, MainView>();
        services.AddTransient<IMainViewModel, MainViewModel>();
        services.AddTransient<IAppSettingProvider, AppSettingProvider>();
        services.AddTransient<ISettingView, SettingView>();
        services.AddTransient<ISettingViewModel, SettingViewModel>();
    })
    .UseSerilog()
    .Build();

var scope = host.Services.CreateScope();
var services = scope.ServiceProvider;
#endregion

#region ***** Run *****
try
{
    Log.Logger.Information("***** Run Application *****");
    Log.Logger.Information($"EnvironmentVariable: {Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT")}");
    Console.WriteLine($"EnvironmentVariable: {Environment.GetEnvironmentVariable("DOTNET_ENVIRONMENT")}");
    Log.Logger.Information($"CommandLineArgument: {builder.Build().GetValue<string>("CommandLineArgument")}");
    Console.WriteLine($"CommandLineArgument: {builder.Build().GetValue<string>("CommandLineArgument")}");
    foreach (var item in args)
    {
        Log.Logger.Information($"Args: {item}");
        Console.WriteLine($"Args: {item}");
    }
    Console.ReadLine();
    services.GetService<IMainView>()!
        .Run(args);
}
catch (Exception e)
{
    Log.Logger.Error("Unexpected Exception!", e);
    Console.WriteLine("Unexpected Exception!");
    Console.WriteLine(e);
    Console.WriteLine($"\n***** Press ENTER To Continue *****");
    Console.ReadLine();
}
finally
{
    Log.Logger.Information("***** End Application *****");

#if DEBUG
    Console.Clear();
    Console.WriteLine($"\n***** End Of Debug Mode - Press ENTER To Close The Window *****");
    Console.ReadLine();
#endif
}
#endregion

MainView.cs (just a little codesnippet, since this class needs to be worked on extensive). When clicking C in a menu this method is called.

    private void Action_C()
    {
        using var scope = _hostProvider.Services.CreateScope();
        var services = scope.ServiceProvider;
        services.GetService<ISettingView>()!.Run();
    }

SettingView here I want to display the information provided by appsettings.json (whether got from the CommandLine, launchSettings.json, appsettings.Production.json or appsettings.json.

using BasicCodingConsole.ConsoleMessages;
using BasicCodingConsole.ConsoleViews;
using BasicCodingLibrary.ViewModels;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Logging;
using System.Diagnostics;

namespace BasicCodingConsole.Views.SettingView;

public class SettingView : ISettingView
{
    #region ***** Field *****
    private readonly ILogger<SettingView> _logger;
    private readonly IConfiguration _configuration;
    private readonly ISettingViewModel _settingViewModel;
    #endregion

    #region ***** Property *****
    public IMessaging StartMessage => new StartingView(nameof(SettingView));
    public IMessaging EndMessage => new EndingView(nameof(SettingView));
    public IView Display => new View();
    #endregion

    #region ***** Constructor *****
    public SettingView(ILogger<SettingView> logger, IConfiguration configuration, ISettingViewModel settingViewModel)
    {
        Debug.WriteLine($"Passing <Constructor> in <{nameof(SettingView)}>.");

        _logger = logger;
        _configuration = configuration;
        _settingViewModel = settingViewModel;
    }
    #endregion

    #region ***** Interface Member (ISettingView) *****
    public void Run()
    {
        Display.Clear();
        Display.Resize(0, 0);
        StartMessage.Show();
        ShowContent();
        EndMessage.Show();
    }
    #endregion

    #region ***** Private Member *****
    private void ShowContent()
    {
        string message = "This is individual text by karwenzman!";
        Console.WriteLine(message);
        _logger.LogInformation(message);

        Console.WriteLine($"\nConnectionString (key = Default): {_configuration.GetConnectionString("Default")}");
    }
    #endregion
}

ISettingView, IConsoleMessage, IConsoleView (each in separate files)

using BasicCodingConsole.ConsoleMessages;
using BasicCodingConsole.ConsoleViews;
namespace BasicCodingConsole.Views.SettingView;
public interface ISettingView : IConsoleMessage, IConsoleView
{
}

namespace BasicCodingConsole.ConsoleMessages;
public interface IConsoleMessage
{
    IMessaging StartMessage { get; }
    IMessaging EndMessage { get; }
}

namespace BasicCodingConsole.ConsoleViews;
public interface IConsoleView
{
    IView Display { get; }
    void Run();
}

IView, View, IClearing and ClearingView (each in separate files)

namespace BasicCodingConsole.ConsoleViews;
public interface IView : IClearing, IResizing
{
}

namespace BasicCodingConsole.ConsoleViews;
public class View : IView
{
    public void Clear()
    {
        IClearing clearingView = new ClearingView();
        clearingView.Clear();
    }

    public void Resize(int consoleWidth, int consoleHeight)
    {
        IResizing resizeView = new ResizingView();
        resizeView.Resize(consoleWidth, consoleHeight);
    }
}

namespace BasicCodingConsole.ConsoleViews;
public interface IClearing
{
    void Clear();
}


namespace BasicCodingConsole.ConsoleViews;
public class ClearingView : IClearing
{
    public void Clear()
    {
        Console.WriteLine($"Calling {nameof(ClearingView)}. Press ENTER to clear the view ...");
        Console.ReadLine();
        Console.Clear();
    }
}

IMessaging and StartingView (each in separate files)

namespace BasicCodingConsole.ConsoleMessages;
public interface IMessaging
{
    string CallingClass { get; }
    void Show();
}

namespace BasicCodingConsole.ConsoleMessages;
public class StartingView : IMessaging
{
    public string CallingClass { get; }

    public StartingView(string callingClass)
    {
        CallingClass = callingClass;
    }

    public void Show()
    {
        string message = $"You have started the view: {CallingClass}";
        Console.WriteLine(message);
        Console.WriteLine("=".PadLeft(message.Length, '='));
    }
}

This is my updated structure.

Class Diagram

I am asking for some mentoring.

\$\endgroup\$
0

2 Answers 2

2
\$\begingroup\$

General Observations

When presenting multiple files of code it is much easier to follow when the file is presented in the following manner:

filename

code in file.

Presenting multiple files like this is very difficult to review the code:

ISettingView, IConsoleMessage, IConsoleView (each in separate files)

using BasicCodingConsole.ConsoleMessages;
using BasicCodingConsole.ConsoleViews;
namespace BasicCodingConsole.Views.SettingView;
public interface ISettingView : IConsoleMessage, IConsoleView
{
}

namespace BasicCodingConsole.ConsoleMessages;
public interface IConsoleMessage
{
    IMessaging StartMessage { get; }
    IMessaging EndMessage { get; }
}

namespace BasicCodingConsole.ConsoleViews;
public interface IConsoleView
{
    IView Display { get; }
    void Run();
}

We generally close questions on code review where we don't see the definitions of objects or methods that are used in the code. This question was closed once for that, there are still objects or interfaces that are used that are not defined, but this question needs a good answer.

Unnecessary Interfaces

It isn't clear that the interface IMainView or the interface ISettingView are necessary, since only once class is derived from each of these interfaces and those classes are not inherited.

public class MainView : ViewBase, IConsoleMessage, IConsoleView
{
}

public class SettingView : IConsoleMessage, IConsoleView
{
}

Use of Regions

There is a lot of use of regions in the code. This is a feature of C#, but it isn't one I use because it tends to hide code that I need to see. Very few of the classes are large enough to require regions to be able to follow the implementation of the object. I generally don't collapse the methods in a class either.

A place where regions obviously are not necessary is Person.cs. This class is very simple and does not fill the screen. If the file was 1000 lines of code and comments regions might possibly make sense (it would still be hiding important code).

using BasicCodingLibrary.Enums;
using System.Reflection;

namespace BasicCodingLibrary.Models;

/// <summary>
/// This class is providing members to describe a <see cref="Person"/>.
/// </summary>
public class Person
{
    #region ***** Property *****
    /// <summary>
    /// The person's unique ID.
    /// </summary>
    public int Id { get; set; } = 0;
    /// <summary>
    /// The person's first name.
    /// </summary>
    public string FirstName { get; set; } = "default";
    /// <summary>
    /// The person's last name.
    /// </summary>
    public string LastName { get; set; } = "default";
    /// <summary>
    /// The person's gender.
    /// </summary>
    public Gender Gender { get; set; } = Gender.unknown;
    #endregion
}

Is the Code SOLID

In the original version of this question you asked if you following the SOLID design principles.

  • The code does follow the Single Responsibility Principle. Based on code from the project that is not posted in this question, the Single Responsibility Principle (SRP) may have been over used, but that isn't completely clear. An example of the overuse of the SRP is in 2 files not presented in the question, Gender.cs and Person.cs. The enum in Gender.cs doesn't seem to be used anywhere in the code that excludes Person.cs so there is no reason to have the enum in a separate file.
  • While there is inheritance of interfaces and classes, it isn't completely clear that the Open-closed Principle is being followed, since there doesn't seem to be any extensions of the classes in use.
  • I don't see any substitution being performed.
  • MainView inherits from ViewBase but there are no other classes that inherit from ViewBase so it isn't clear that this distinction is necessary.

I suggest that you do some more research on this part of your question by using the links provided here:

SOLID is 5 object orieneted design principles. SOLID is a mnemonic acronym for five design principles intended to make software designs more understandable, flexible and maintainable. This will help you design your objects and classes better.

  1. The Single Responsibility Principle - A class should only have a single responsibility, that is, only changes to one part of the software's specification should be able to affect the specification of the class.
  2. The Open–closed Principle - states software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
  3. The Liskov Substitution Principle - Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.
  4. The Interface segregation principle - states that no client should be forced to depend on methods it does not use.
  5. The Dependency Inversion Principle - is a specific form of decoupling software modules. When following this principle, the conventional dependency relationships established from high-level, policy-setting modules to low-level, dependency modules are reversed, thus rendering high-level modules independent of the low-level module implementation details.
\$\endgroup\$
6
  • \$\begingroup\$ Thanks! For now I will update my post to follow your general observations. It will take some time, but I am on it. \$\endgroup\$
    – karwenzman
    Jul 10, 2023 at 7:28
  • \$\begingroup\$ @karwenzman Do not update this post. You can post a follow up question with a link back to this question. \$\endgroup\$
    – pacmaninbw
    Jul 10, 2023 at 12:34
  • 1
    \$\begingroup\$ follow up is online click here \$\endgroup\$
    – karwenzman
    Jul 11, 2023 at 16:28
  • \$\begingroup\$ Unnecessary interfaces - YES! And it just creates avoidable problems when we ignore classes' natural polymorphic capabilities. Everything is public when often some things should not be; corrupting interface segregation principle for want of proper class design; no base class code templating, guaranteeing problematic code variations; superfluous interfaces cost maintenance time just like every line of code but with no benefit. It's all a corruption of interface intended purpose, to give unrelated classes a common polymorphic behavior that they otherwise do not have. \$\endgroup\$
    – radarbob
    Jul 29, 2023 at 20:50
  • \$\begingroup\$ @radarbob You should probably write an answer. \$\endgroup\$
    – pacmaninbw
    Jul 30, 2023 at 14:24
0
\$\begingroup\$

I don’t know anything about C#, so I can’t really review your code. But I just feel the need to say here that code like

IClearing clearingView = new ClearingView();
clearingView.Clear();

Should just be

ClearView();

(or whatever name you want to give it.) This is more concise, less repetitive, and easier to read.

A class that doesn’t own any data is rather pointless in my opinion. Because the methods of the class can just as easily be free functions. They have no class data to act on, they have no status to modify.

Without a clear understanding of what your code accomplishes, just going by your description of it, I would expect all of this code to become three or four very short functions, if you were to remove all these classes that, to me, don’t seem to have a purpose.

Your goal is to write some infrastructure for a console application, but you are adding lots of stuff just in case, in the future, maybe you decide to extend the infrastructure to support a different UI. Well, all that stuff is wasted effort if you end up not extending it, which is the most likely outcome (see the YAGNI principle). Not only is it more work to implement, but it is also more work to maintain. If you keep it simple, it will be quite easy to replace the few functions with a different set that operates on a different UI, if you ever need that.

\$\endgroup\$
4
  • \$\begingroup\$ Thanks for highlighting the YAGNI principle. One comment suits perfectly to me: John Carmack wrote "It is hard for less experienced developers to appreciate how rarely architecting for future requirements / applications turns out net-positive." \$\endgroup\$
    – karwenzman
    Jul 9, 2023 at 8:21
  • \$\begingroup\$ So far this is just a training application. And with more experience I will surely consolidate a lot of things. Since I am trying to understand the SOLID principles I am overkilling a lot of things. In the mentioned code snippet I was trying to implement the class to be open for extension, but closed for modification. Creating a new class that is implementing IView and add a new instance of a class that implements IClearing should make it possible to extend the app, without modificate existing classes. This was the goal, maybe I have messed it up. \$\endgroup\$
    – karwenzman
    Jul 9, 2023 at 8:36
  • \$\begingroup\$ @karwenzman I don’t know if you messed it up. Bloated code and chasing across many files after the bit of code that actually does the work is typical of OOP code. So to me your implementation looks quite typical. I don’t like OOP because of that. Maybe there are projects that are better off because of OOP, but IMO most are not. This is a personal opinion though. I’d encourage you to try implementing this without any classes, and compare the two implementations. Maybe you do prefer the OOP one, maybe that just is more natural to you. Don’t be convinced by a stranger online, convince yourself! \$\endgroup\$ Jul 9, 2023 at 13:08
  • \$\begingroup\$ To some extent OOP is limited by language, you can't write OOP in C, I have tried. What you can do is write modular code. The point of OOP is to have as much reusable code as possible, that is also the point of modular code in C. Well written objects should be reusable in multiple programs or implementations. The same is true for well written procedural code. \$\endgroup\$
    – pacmaninbw
    Jul 9, 2023 at 13:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.