CODEWITHSUNDEEP
c#.netcoding-style
csharptest.net
csharptest.net

Reputation: 64218

Thoughts on the use of readonly for any/all instance members of a class?

Preface

For some time now I've been using the readonly modifier for nearly all class fields. I use it for List<T> members, IDisposeable members, ints, strings, etc... everything but value types I intend to change. I tend to do this even when I would normally like to null a member on Dispose(). IMHO The advantages of not needing the if statements to test for null or disposed conditions greatly outweigh the 'potential' for trouble in objects that 'could' be disposed multiple times.

The Question

When do you use readonly, or do you?

Do you or your company have any best-practices and/or coding standards regarding the use of readonly?

I'm curious to hear your thoughts on the following sample class, is the general concept a good practice or not? 

class FileReaderWriter : IFileReaderWriter, IDisposable
{
    private readonly string _file;
    private readonly Stream _io;

    public FileReaderWriter(string path) 
    {
        _io = File.Open(_file = Check.NotEmpty(path), FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
    }
    public void Dispose() { _io.Dispose(); }
    ...
}

Upvotes: 8

Views: 1671

Answers (6)

Brian Rasmussen
Brian Rasmussen

Reputation: 116411

Like several other repliers I use readonly very frequently. Fortunately Resharper is very keen on spotting when members can be made readonly. It shows the reader of the code, that this value is set once the instance is created, which is a valuable detail imo.

Upvotes: 0

Allain Lalonde
Allain Lalonde

Reputation: 93348

For the sake of shorter code, I don't put it in, but only if I have a static analysis tool that will guard against misuse.

For example when writing in Java I rely on PMD to make sure I'm not shooting myself in the foot by not declaring all my parameters as final.

You could, correctly, argue that using readonly/final will inform you of your error sooner, but I've been doing it long enough that my spidey sense goes off while writing, and if I happen to miss something the static analysis tool reminds me of it.

Upvotes: 0

womp
womp

Reputation: 116977

Our company does not have any official coding standards about readonly, but it's universally agreed on our team to use it whenever possible and where it makes sense (which is often). It clearly indicates the intention of the programmer or the spec that this wasn't meant to be changed once it's set.

A few of us also use Resharper, which tends to remind you to use it quite a bit. So your example class would be something that would be considered a "good practice" on our team.

Upvotes: 0

JaredPar
JaredPar

Reputation: 754833

I use readonly on fields in any situation where the code will successfully compile.

Why? Simple, if a field reference / value suddenly goes from never changing to changing it can violate subtle assumptions in the class and consumers. As such I want to be alerted to that change in order to evaluate the implications of this new behavior.

Upvotes: 10

Alex
Alex

Reputation: 77329

The readonly keyword is just another tool in your .NET toolbox. Therefore use it where it applies! In general you want to great the least amount of access to properties,

The reason is that you want to protect your code from being called/accessed from places where you didn't intend it to be accessed from. So going back to your readonly question, if you have class member that are only to be modified (set) once from the constructor, then readonly is the way to do this.

Upvotes: 3

Jon Skeet
Jon Skeet

Reputation: 1500873

Like you, I use readonly everywhere I can. Where possible, I use immutable collections too.

Dispose is an interesting one, because it's effectively mutating the type from "usable" to "unusable" - I'd be tempted to have a non-readonly bool member to indicate that. Then again, I rarely find myself needing to implement IDisposable. Usually when I use resources, it's only for the course of a single method, so I make the resource local to that method.

Upvotes: 5

Related Questions