0

I have been reading the book Clean Code and it says that function arguments should not more than 2, anything more than that is confusing for anyone using my function. My question is, how does this apply to immutable classes?. Lets say for example that I have something like the following:

public class Song
{
    private readonly string name;
    private readonly double price;
    private readonly string owner;
    private readonly int id;

    public string Name{get{return name;}}
    public double Price{get{return price;}}
    public string Owner{get{return owner;}}
    public int Id{get{return id;}}

    public Song(string name, double price, string owner, int id)
    {
        this.name = name;
        this.price = price;
        this.owner = owner;
        this.id = id;
    }
}

I have 4 parameters in my constructor and this does not look clean, is there a better way to create an immutable class? or maybe im putting too much thought into it and should not worry about this.

Teler
  • 477
  • 6
  • 15
  • 6
    Throw away the book. I have seen a lot of discussions on Guidelines of writing code and never seen any recommendations of limiting parameter lists to only two items. – jdweng Oct 18 '18 at 20:31
  • 1
    That's a stupid rule, well, guideline... There will be plenty of times where > 2 parameters are necessary (and still very much "clean code"). Your example is perfectly fine. Now, if you start having classes/methods taking 100 parameters instead, then you're probably doing something wrong. – Broots Waymb Oct 18 '18 at 20:33
  • Just FYI C# 8's records will make this process faster – Konrad Oct 18 '18 at 20:34
  • 2
    @jdweng Clean Code is one of the most well-known and highly praised books in the software development field. You should probably give the book a read before dismissing it outright. – Lews Therin Oct 18 '18 at 20:35
  • check this link https://softwareengineering.stackexchange.com/questions/151733/if-immutable-objects-are-good-why-do-people-keep-creating-mutable-objects – Rudresha Parameshappa Oct 18 '18 at 20:40
  • 2
    I note that in recent versions of C# you can reduce some of that boilerplate code. `public string Name { get; }` is equivalent to declaring a readonly field and having a setter that is only accessible in the ctor. – Eric Lippert Oct 18 '18 at 20:47
  • According to *Code Complete, 2nd ed.*, page 178: "Limit the number of a routine's parameters to about seven." –  Oct 18 '18 at 20:48
  • 5
    Also **NEVER EVER USE DOUBLE TO REPRESENT MONEY**. Use doubles to represent physical quantities like weight or distance. Use `decimal` to represent money. Doubles cannot exactly represent decimals; that's why we have `decimal`. – Eric Lippert Oct 18 '18 at 20:49
  • @jdweng The book was recommended by my boss and everyone I have talked to praises the book, maybe its out dated? I dont know. – Teler Oct 18 '18 at 20:59
  • @EricLippert I was not even aware of decimal, so if double is for physical quantities and decimal for money, when should I use float? – Teler Oct 18 '18 at 21:01
  • 1
    @Teler Check this question: [Difference between decimal, float and double in .NET?](https://stackoverflow.com/questions/618535/difference-between-decimal-float-and-double-in-net) – aloisdg Oct 18 '18 at 21:03
  • 2
    @Teler: You shouldn't use `float`. It's almost always the wrong thing to use. `float` is there for (1) backwards compatibility with ancient machines, and (2) the rare scenarios where you have huge amounts of low-precision physical data and don't want to double your storage requirements. That is why doubles are called doubles: because they take double the space of a float. In 1972, four bytes extra bytes was expensive. In 2018, storage is literally billions of times cheaper. – Eric Lippert Oct 18 '18 at 21:07
  • 2
    If someone tells you that floats are cheaper, well, they're probably very wrong. In modern hardware, all floating point arithmetic whether it is 4 bytes or 8 bytes is done in high-precision hardware registers and the chip then has to do extra work to convert it back to 4 or 8 bytes. The only savings you get with floats is, as I said, the storage requirements. – Eric Lippert Oct 18 '18 at 21:09
  • @aloisdg thanks a lot for the link, I actually did read that answer but I completely forgot I did hahaha, memory can be tricky sometimes. – Teler Oct 18 '18 at 21:10
  • @EricLippert Yeah you are right, storage nowadays is not a problem, I was just used to float and double, gonna start to use decimal more. Just a question, if decimal is more precise why is not used also in physical data? – Teler Oct 18 '18 at 21:16
  • 1
    @Teler: Decimal is much larger, which as you note is not so bad. What's bad is it is much *slower*. Chips are heavily optimized for double arithmetic. Also, think about what you just said. Decimal is precise to 29 decimal places. **What physical quantity can you name that can be measured accurately to 29 decimal places**? That's enough precision to measure the distance from here to Proxima Centauri to a precision of *0.1% of the width of an atom*. There are almost no physics computations that require anywhere near that precision! – Eric Lippert Oct 18 '18 at 21:24
  • Basically, with decimals we trade slightly slower, larger arithmetic for guaranteeing that we never round off a penny wrong. With doubles we get much larger range, precision appropriate for physics, high speed computations, and the price we pay is some rounding errors for decimal quantities. – Eric Lippert Oct 18 '18 at 21:26
  • @EricLippert Maybe if we need to compare the speed of neutrinos with the speed of light(Joking about the OPERA team) haha but I dont think even that needs that much precision. Thanks a lot for the answers, I came here to clear my head about constructors and left with a lot of knowledge about decimal points. – Teler Oct 18 '18 at 21:34
  • 1
    Also, to address the central point here: your code looks perfectly clean to me. The "ceremony" attendant to "record" types like this is just an unfortunate historical quirk of how the language evolved. There's been a proposal for many years now to reduce this ceremony to something a little less heavyweight. I would *hope* that it gets into C# 8, but I would not personally *expect* it. (Remember, Eric's speculations on future language features are for amusement purposes only.) – Eric Lippert Oct 18 '18 at 22:01
  • @EricLippert Awesome, hopefully it gets into c# 8, would love that. – Teler Oct 18 '18 at 23:05
  • 1
    @Teler: Beyond the space/precision differences between decimal and float/double, the big technical difference is that decimals are stored in base 10. Binary floating point numbers cannot represent 0.1 without rounding. – Brian Oct 19 '18 at 13:01

2 Answers2

0

Uncle Bob recommends that functions (methods) should ideally have no arguments. But, when there is a need, keep them under two unless there is a good reason for a third. Any more than three is a code smell.

The reasoning behind this is that your functions should be small and simple. This makes them easy to read and understand (i.e. "clean"). If you have more than three arguments, the method is most likely doing more than one thing. Split it up into separate methods.

That being said, I don't think his advice on limiting the number of parameters applies to class constructors. The constructor isn't supposed to really do anything besides initialize fields or properties.

Constructors have the same name as the class or struct, and they usually initialize the data members of the new object.

There isn't much complexity to simply initializing fields/properties. I'd say your code is still "clean".

Edit: As Eric Lippert pointed out in the comments, you could simplify your code with auto-implemented properties:

public class Song
{
    public string Name { get; }
    public decimal Price { get; }
    public string Owner { get; }
    public int Id { get; }

    public Song(string name, decimal price, string owner, int id)
    {
        this.Name = name;
        this.Price = price;
        this.Owner = owner;
        this.Id = id;
    }
}
Lews Therin
  • 3,707
  • 2
  • 27
  • 53
0

You need to pass in the parameters in the constructor of your immutable class. So if you have a lengthy class, there will be many parameters. You can create a copy-constructor if this feels cleaner, but the instance you pass in will again need to be defined using the lengthy constructor variant, passing in each parameter to map to an instance field or property. We can compact the code using C# 6.0 readonly properties at least too. Give the following code a go in Linqpad and you will see Linqpad inform that we cannot set the song from "Thunderstruck" to "Big Gun", since we declared the property to be readonly. Less ceremony, less code, that is what we like. Perhaps we could do something elegant with expression trees to perhaps compact the code even more? This is my suggestion at least. 

void Main()
{
    var song = new Song("Thunderstruck", 15, "AC/DC Records",
        123
    ); 
    var songAwesome = new Song(song);
    songAwesome.Name.Dump();
    songAwesome.Name = "Big Gun"; //This line fails because the property is redonly

}

public class Song
{

    public string Name { get; } //readonly props in C# 6.0
    public double Price { get;}
    public string Owner { get; }
    public int Id { get; }

    public Song(Song song) : this(song.Name, song.Price, 
    song.Owner, song.Id){

    } //Let us have a copy constructor just for the fun of it

    public Song(string name, double price, string owner, int id)
    {
        this.Name = name;
        this.Price = price;
        this.Owner = owner;
        this.Id = id;
    }
}


// Define other methods and classes here
Tore Aurstad
  • 3,189
  • 1
  • 27
  • 22