patterncsharpMinor
FizzBuzz without hard-coded logic checks
Viewed 0 times
withoutcheckshardlogicfizzbuzzcoded
Problem
I'm a self-taught (on the job) programmer, always looking for ways to expand my skills. Seeing all the FizzBuzz examples with hard-coded logic checks made my head hurt, so I thought I'd try my hand at it. Leaning heavily on KISS, I'm ignoring exception handling and such, and all that's required to add new fizzbuzzers is a call to Catalog.Add(Divisor,Output).
I'd appreciate your feedback, whether on style or substance. This is part of my journey to adopting SOLID principles for better OO development, but I felt that not much could be applied here without over-complicating it.
.NetFiddle
I'd appreciate your feedback, whether on style or substance. This is part of my journey to adopting SOLID principles for better OO development, but I felt that not much could be applied here without over-complicating it.
using System;
using System.Collections.Generic;
public class Program
{
public static void Main()
{
var catalog = new Catalog();
catalog.Add(Divisor: 3, Output: "Fizz");
catalog.Add(Divisor: 5, Output: "Buzz");
catalog.Add(Divisor: 10, Output: "Pozz");
var counter = new Counter(Min: 1, Max: 100, Catalog: catalog);
counter.Output();
Console.ReadLine();
}
}
internal class Counter
{
private int min;
private int max;
private Catalog catalog;
internal Counter(int Min, int Max, Catalog Catalog)
{
this.min = Min;
this.max = Max;
this.catalog = Catalog;
}
internal void Output()
{
for(int i = min; i specs;
internal class Spec
{
internal int divisor;
internal string output;
}
internal Catalog()
{
this.specs = new List();
}
internal void Add(int Divisor, string Output)
{
this.specs.Add(new Spec() { divisor = Divisor, output = Output });
}
internal string ToString(int Number)
{
string outputstring = "";
foreach (var x in specs)
{
outputstring += Number % x.divisor == 0 ? x.output : "";
}
return String.IsNullOrWhiteSpace(outputstring) ? Number.ToString() : outputstring;
}
}.NetFiddle
Solution
The code looks well structured and seems to do its job (I have just copy-pasted it in VS2015 and worked directly), so many of the above are rather superficial:
1) Function (including constructor) parameter naming. In C#, usually all function parameters are named using CamelCase.
2) Non-private members naming. Here, I would also favor
3) ToString usage -
Also, since strings are immutable, concatenation is faster and allocates less memory if
}
The whole code:
1) Function (including constructor) parameter naming. In C#, usually all function parameters are named using CamelCase.
internal Counter(int min, int max, Catalog catalog)
{
this.min = min;
this.max = max;
this.catalog = catalog;
}2) Non-private members naming. Here, I would also favor
PascalCase over CamelCase, although this targeted question and answers do not seem to clarify this.internal class Spec
{
internal int Divisor;
internal string Output;
}3) ToString usage -
ToString(int Number) function has exactly the same name as Object.ToString() function (no parameters). Although it is perfectly legal (becomes an overload), I would choose a different name, just not to create any confusion (your are not getting a string representation of the object using its state only, but also providing some input).Also, since strings are immutable, concatenation is faster and allocates less memory if
StringBuilder is used. internal string GetString(int Number)
{
var builder = new StringBuilder();
foreach (var x in specs)
{
builder.Append(Number % x.Divisor == 0 ? x.Output : "");
}
string outputString = builder.ToString();
return String.IsNullOrWhiteSpace(outputString) ? Number.ToString() : outputString;
}}
The whole code:
using System;
using System.Collections.Generic;
using System.Text;
public class Program
{
public static void Main()
{
var catalog = new Catalog();
catalog.Add(divisor: 3, Output: "Fizz");
catalog.Add(divisor: 5, Output: "Buzz");
catalog.Add(divisor: 10, Output: "Pozz");
var counter = new Counter(Min: 1, Max: 100, Catalog: catalog);
counter.Output();
Console.ReadLine();
}
}
internal class Counter
{
private int min;
private int max;
private Catalog catalog;
internal Counter(int min, int max, Catalog catalog)
{
this.min = min;
this.max = max;
this.catalog = catalog;
}
internal void Output()
{
for (int i = min; i specs;
internal class Spec
{
internal int Divisor;
internal string Output;
}
internal Catalog()
{
specs = new List();
}
internal void Add(int divisor, string Output)
{
specs.Add(new Spec() { Divisor = divisor, Output = Output });
}
internal string GetString(int Number)
{
var builder = new StringBuilder();
foreach (var x in specs)
{
builder.Append(Number % x.Divisor == 0 ? x.Output : "");
}
string outputString = builder.ToString();
return String.IsNullOrWhiteSpace(outputString) ? Number.ToString() : outputString;
}
}Code Snippets
internal Counter(int min, int max, Catalog catalog)
{
this.min = min;
this.max = max;
this.catalog = catalog;
}internal class Spec
{
internal int Divisor;
internal string Output;
}internal string GetString(int Number)
{
var builder = new StringBuilder();
foreach (var x in specs)
{
builder.Append(Number % x.Divisor == 0 ? x.Output : "");
}
string outputString = builder.ToString();
return String.IsNullOrWhiteSpace(outputString) ? Number.ToString() : outputString;
}using System;
using System.Collections.Generic;
using System.Text;
public class Program
{
public static void Main()
{
var catalog = new Catalog();
catalog.Add(divisor: 3, Output: "Fizz");
catalog.Add(divisor: 5, Output: "Buzz");
catalog.Add(divisor: 10, Output: "Pozz");
var counter = new Counter(Min: 1, Max: 100, Catalog: catalog);
counter.Output();
Console.ReadLine();
}
}
internal class Counter
{
private int min;
private int max;
private Catalog catalog;
internal Counter(int min, int max, Catalog catalog)
{
this.min = min;
this.max = max;
this.catalog = catalog;
}
internal void Output()
{
for (int i = min; i <= max; i++)
{
Console.WriteLine(catalog.GetString(i));
}
}
}
internal class Catalog
{
private List<Spec> specs;
internal class Spec
{
internal int Divisor;
internal string Output;
}
internal Catalog()
{
specs = new List<Spec>();
}
internal void Add(int divisor, string Output)
{
specs.Add(new Spec() { Divisor = divisor, Output = Output });
}
internal string GetString(int Number)
{
var builder = new StringBuilder();
foreach (var x in specs)
{
builder.Append(Number % x.Divisor == 0 ? x.Output : "");
}
string outputString = builder.ToString();
return String.IsNullOrWhiteSpace(outputString) ? Number.ToString() : outputString;
}
}Context
StackExchange Code Review Q#121714, answer score: 4
Revisions (0)
No revisions yet.