write(this); //not that

… from what I’ve been seeing lately, this horse isn’t quite dead yet.  As a rule of thumb, if your method is so long that you have to scroll through to take in the whole thing, then you might need to consider refactoring.  Long methods make your code more difficult to read and consequently, more difficult to maintain.  You wouldn’t use a run-on sentence, would you?  Consider the following method from an application I worked on a few years ago:

        /// <summary>
        /// Validate a new request.
        /// the request to validate
        /// Current options for processing the request
        /// thrown if the request does not pass validation
        internal void ValidateNewRequest(EntitlementAccountRequest request)
        {
            // An admin account must have an owner
            if (!request.Account.AccountTypeReference.IsLoaded)
                request.Account.AccountTypeReference.Load();

            if (request.Account.AccountType.AccountTypeID == Enums.AccountType.AdminAccount)
            {
                Check.Require(
                    (DataHelper.GetReferencedEntity(request.Account.OwnerReference) != null),
                    "Admin Account must have an Owner");
            }

            // IF request bypasses approvals AND CurrentPerson is NOT Admin*...
            // * The check for Admin has been specifically implemented here
            // to handle the MassRequest Manual Import functionality.
            // This is the only place where the Entitlement's IsAllowBypassApprovals
            // property is not checked AND the User may bypass approvals for accounts they own.
            if (request.IsBypassApprovals &&
                !RodanRoles.IsCurrentPersonAdmin())
            {
                if (request.Account.Owner != null)
                    Check.Require(request.Requester.PersonID != request.Account.Owner.PersonID,
                                  "Users may not bypass approvals for accounts they own.");
            }

            // validate account does not already have a open request for the entitlement.
            List openRequests =
                GetPendingByAccountID(request.Account.AccountID, true, EntitlementAccountRequest.ENTITLEMENT);
            foreach (EntitlementAccountRequest openRequest in openRequests)
            {
                if (openRequest.Entitlement == request.Entitlement)
                {
                    Check.Assert(false,
                        string.Format("Open request exists for {0}.",
                            openRequest.Account.InformalDisplayDescription));
                }
            }

            // validate account does not already have a map to this request IF we are granting
            if (request.RequestType.RequestTypeID == Enums.RequestType.Grant)
            {
                List existingMaps =
                    BllFactory.EntitlementAccountMapBll.GetCurrentByAccountIDandEntitlementID(
                        request.Account.AccountID,
                        request.Entitlement.EntitlementID
                        );
                Check.Assert(
                    (existingMaps.Count == 0),
                    "Account already has a current (granted) request for this entitlement."
                    );
            }

            if (!request.Entitlement.ApprovalTypeEntitledPartyMaps.IsLoaded)
                request.Entitlement.ApprovalTypeEntitledPartyMaps.Load();
            // ensure that a supervisor exists if one will be required to provide approval.
            // this applies only to SecurePerson recipients who are not users.
            DataHelper.GetReferencedEntity(request.Account.OwnerReference);
            SecurePerson recipientOwner = request.Account.Owner;

            if (!request.IsBypassApprovals
                && (recipientOwner != null)
                && (!(recipientOwner.IsOfficer
                    || (recipientOwner.ArchonHRID == null))
                ))
            {
                foreach (EntitlementApprovalEntitledPartyMap map in request.Entitlement.ApprovalTypeEntitledPartyMaps)
                {
                    EntitledPartyType partyType = DataHelper.GetReferencedEntity(map.EntitledPartyTypeReference);
                    EntitlementApprovalType approvalType =
                        DataHelper.GetReferencedEntity(map.EntitlementApprovalTypeReference);
                    if ((partyType.EntitledPartyTypeID == Enums.EntitledPartyType.Account) &&
                        (approvalType.EntitlementApprovalTypeID == Enums.EntitlementApprovalType.Supervisor))
                    {
                        Check.Require(DataHelper.GetReferencedEntity(recipientOwner.SupervisorReference) != null,
                            "Account Owner must have a supervisor for non-Direct Execute requests.");
                    }
                }
            }
        }

So, granted, the following method is pretty easy to understand with all of the comments, but it does sort of ramble.  I mean, it seems like we’re doing a lot of work for one method.  It might make more sense to split out the functionality into their own seperate methods.  Let’s take a look and see what we can do.

        /// <summary>
        /// Validate a new request.
        /// the request to validate
        /// Current options for processing the request
        /// thrown if the request does not pass validation
        internal void ValidateNewRequest(EntitlementAccountRequest request)
        {
            // 1. An admin account must have an owner
            VerifyOwnership(request); 

            // 2. IF request bypasses approvals AND CurrentPerson is NOT Admin*...
            // * The check for Admin has been specifically implemented here
            // to handle the MassRequest Manual Import functionality.
            // This is the only place where the Entitlement's IsAllowBypassApprovals
            // property is not checked AND the User may bypass approvals for accounts they own.
            VerifyThatApprovalBypassIsAllowed(request);

            // 3. validate account does not already have a open request for the entitlement.
            VerifyThatOpenRequestsDoNotExist(request);

            // 4. validate account does not already have a map to this request IF we are granting
            VerifyNotAlreadyGranted(request);

            // 5. ensure that a supervisor exists if one will be required to provide approval.
            // this applies only to SecurePerson recipients who are not users.
            VerifyThatSupervisorExists(request);
        }

Although there are still a few too many comments for my personal taste, I think that this method is much easier to read and fully comprehend.  It’s also easier to maintain for the next developer that has to modify your code.  Like I said before, if you have to scroll to read the entire method, consider breaking it out.  There are a few cases that I’ve seen where that’s more difficult to do, like an exceptionally long switch statement, but, even still, you may want to reconsider refactoring.