DEV Community

Discussion on: Two quick tips for writing legible code

Collapse
 
peledzohar profile image
Zohar Peled

IMHO, the method names in the last code example are way too long. Personally, I find such long names hard to read, even though I know that the capital letter means a new word.
Instead of these huge long method names, use XML documentation comments.
If it's a public API, compile it with the XML documentation file.

I would much rather read this code instead, even though I've made only small, seemingly insignificant changes to your example:

namespace JEasthamDev
{
    /// <summary>
    /// Responsible for uploading outstanding documents to ocr engine.
    /// </summary>
    public class Uploader 
    {
        /// <summary>
        /// Uploads all outstanding documents to ocr engine.
        /// </summary>
        public void UploadAll()
        {
            var outstandingDocuments = await this._documentService.GetDocuments();
            var batchName = this.GenerateBatchName();

            foreach (var document in outstandingDocuments)
            {
                this.UploadSingle(batchName, document);
            }
        }

        private void UploadSingle(string batchName, IDocument document)
        {
            foreach (var file in document.GetFilesNotSentForOcr())
            {
                var ocrBatch = this.GetOrCreateBatch(batchName);

                ocrBatch.AddFileFromBytes(file.FileName, file.GetByteRepresentation());

                ocrBatch.Update();
            }
        }

        private IBatch GetOrCreateBatch(string batchName)
        {
            using (var ocrResource = new OcrResource)
            {
                var existingBatch = ocrResource.GetBatch(batchName);

                if (existingBatch == null)
                {
                    existingBatch = ocrResource.CreateBatch(batchName);
                }

                existingBatch.AddFiles(document.FileName, bytes);

                ocrResource.UpdateBatch(batch);
            }
        }

        private string GenerateBatchName(){
            return $"NewBatch_{DateTime.Now.ToString("yyyyMMdd-HHmmss")}";
        }
    }
}
Collapse
 
jeastham1993 profile image
James Eastham

Yeah, I do like the casing changes. However, I like keeping private methods distinguishable in my code by keeping the first letter lowercased.

Also, agree with your point on the long method names, that's something I've mulled over for quite a long time and probably the one thing I can never decide on a good length for.

I find a nice descriptive method name makes a high level skim read of the code much easier. XML comments are nice, but it means you need to interrupt the reading flow by hovering the mouse over the method name.

I can cope with the length, if it gives enough detail to be useful. Obviously, something like

public void UploadDocumentsAndCreateBatchInOcrIfDoesNotExist(IDocument document){

}

is just crazy and something I would never condone.

That said, if I was writing a Nuget library or anything open source I would add XML comments on all public API's.

Collapse
 
peledzohar profile image
Zohar Peled • Edited

Well, in that case, I would rather call the class itself "OutstandingDocumentsToOcrEngine" and leave that part out of the upload methods names - but hey, that's just me.
I find it very hard to read these long names, and I don't think it's because I'm a non-native English speaker, (after all, I've been reading and writing PascalCase/camelCase code for over two decades now).

I mean, compare how easy it is to read

Upload documents and create batch in ocr if does not exist

vs

UploadDocumentsAndCreateBatchInOcrIfDoesNotExist

It's not just me, right?

Thread Thread
 
jeastham1993 profile image
James Eastham

Fantastic points, especially on making it a class instead of a set of methods. It's not you at all!

My example may be trivial, but the crux of what I was trying to get across is small methods with descriptive names are better than one long method which is difficult to follow.

I put this post out hoping to start a discussion and that's exactly what we've got. :)

Thread Thread
 
peledzohar profile image
Zohar Peled

You might also want to read Stop Writing Code Comments. I can't say I totally agree with this, since I really do believe that comments should be used in code (Especially xml documentation for public APIs, but not only). In fact, in a comment to another post a few days ago I was telling how good commenting habits from the old me saved the current me literally days of work - but yeah, as a rule of thumb, he does have a valid point: good code is a code that's easy to read even without comments.

Thread Thread
 
jeastham1993 profile image
James Eastham

I agree wholeheartedly with the zero comments within code. I think if you need to add a comment to describe a line of code, then the line of code isn't as clean as it could be.

var documentList = this._service.Get(); // Retrieve all outstanding documents from the database

foreach (var document in documentList){

}

vs

var outstandingDocuments = this._documentService.GetOutstandingDocuments();

foreach (var outstandingDocument in outstandingDocuments){

}

The second one is longer, but I'm completely ok with that for the gain in legibility.

Public API's are a whole different matter, and as far as I'm concerned should always be fully documented with XML comments.