is it better to test if a function is needed inside or outside of it?

Question

what is the best practice? call a function then return if you test for something, or test for something then call?

i prefer the test inside of function because it makes an easier viewing of what functions are called.

for example:

protected void Application_BeginRequest(object sender, EventArgs e)
        {
            this.FixURLCosmetics();
        }

and

private void FixURLCosmetics()
        {
            HttpContext context = HttpContext.Current;
            if (!context.Request.HttpMethod.ToString().Equals("GET", StringComparison.OrdinalIgnoreCase))
            {
                // if not a GET method cancel url cosmetics
                return;
            };

            string url = context.Request.RawUrl.ToString();
            bool doRedirect = false;

            // remove > default.aspx
            if (url.EndsWith("/default.aspx", StringComparison.OrdinalIgnoreCase))
            {
                url = url.Substring(0, url.Length - 12);
                doRedirect = true;
            }

            // remove > www
            if (url.Contains("//www"))
            {
                url = url.Replace("//www", "//");
                doRedirect = true;
            }

            // redirect if necessary
            if (doRedirect)
            {
                context.Response.Redirect(url);
            }
        }

is this good:

if (!context.Request.HttpMethod.ToString().Equals("GET", StringComparison.OrdinalIgnoreCase))
            {
                // if not a GET method cancel url cosmetics
                return;
            };

or should that test be done in Application_BeginRequest?

what is better?

thnx

Answer 1

I feel like testing inside the function is better. If you test outside of the function, you'll have to test everywhere that function could be called (and would cause a lot of duplicate code).

It's nicer to have everything in one place then spread out everywhere.

Answer 2

If a method absolutely requires a certain condition to be met before it can perform its function then yes, you should put the validation inside that function. If, on the other hand, your calling code is saying "only perform this operation under this set of conditions" then the condition is better in the calling code, because next time you want to call that method you may not want to include that condition.

Answer 3

In this case, I feel that the name of the function implies that something is going to happen to the URL in every case. Someone may want to call FixURLCosmetics on a non-GET page and expect something to happen.

I would rename FixURLCosmetics to FixGETURLCosmetics. Then, throw an exception if it's called on a non-GET page.

Answer 4

If I were you i would test in BOTH places, being outside AND inside, and mocking the inner components that are being called (such as context.Request calls), to strengthen also the inner behavior and also mocking some unexpected returns and how your method deals with them.

In this case, an API such as easymock could simplifty A LOT the mocking of inner components.