Wasteful processing of collections-I want to give you a chance to write good code. 5 [C # refactoring sample]

Wasteful processing of collections

For collections, we often see the coding convention "collections return empty instances without returning null" in normal projects. On the other hand, many people heard this agreement for the first time. As a result, we often see useless null checks. Also, when a Java experienced person helped me with a C # project, there was a person who checked the number of cases and then did a for loop. I feel that the collection operation is not well known regardless of language.

Sample code

Consider the code that gets from the DB and outputs the obtained value to the console. There are some issues with this code.

  1. The collection is supposed to be null.
  2. If the collection is null, an "error" is output, but the exception information is squeezed.
  3. Check the number of cases before using foreach.
          internal void Main()
        {
            var items = GetDataFromDB();
            //NG1 NULL check before foreach
            if (items != null)
            {
                foreach (var item in items)
                {
                    Console.WriteLine(item);
                }
            }

            //NG2 NULL check and count check before foreach
            if (items != null && items.Count > 0)
            {
                foreach (var item in items)
                {
                    Console.WriteLine(item);
                }
            }

            //I'm assuming that NG3 NULL will not come, but for some reason check the number of cases
            if (items.Count > 0)
            {
                foreach (var item in items)
                {
                    Console.WriteLine(item);
                }
            }

            //Error if NG4 list is null
            if (items == null)
            {
                Console.WriteLine("error");
            }
        }

        /**Get data from DB**/
        private List<string> GetDataFromDB()
        {
            //Get data from DB and set NULL in case of error.
            //The new part is assumed to be getting data from Dao.
            try
            {
                var list = new List<string>();
                list.Add("1");
                list.Add("2");
                return list;
            }
            catch (Exception)
            {
                return null;
            }
        }

After refactoring

Removed the assumption of null and included the exception in the return value.

        internal void Main()
        {
            var result = GetDataFromDB();
            var items = result.Items;
            //No null check or count check required
            foreach (var item in items)
            {
                Console.WriteLine(item);
            }

            //If an exception has occurred, the exception information is output.
            if (result.Exception !=null)
            {
                Console.WriteLine(result.Exception.ToString());
            }
        }

        /**Get data from DB**/
        private Result GetDataFromDB()
        {
            //Get data from DB and set NULL in case of error.
            //The new List part is assumed to be getting data from Dao.
            try
            {
                var list = new List<string>();
                list.Add("1");
                list.Add("2");
                return new Result(list, null);
            }
            catch (Exception ex)
            {
                return new Result(new List<string>(), ex);
            }
        }

        /**Processing result class including return value and exception handling**/
        private class Result
        {
            internal List<string> Items { get; private set; }
            internal Exception Exception { get; private set; }

            internal Result(List<string> items, Exception ex)
            {
                this.Items = items;
                this.Exception = ex;
            }
        }

Summary

By not making the collection null, we were able to eliminate redundant code. However, for those who implement "check the number of cases before foreach", it is necessary to consider another method. Mysteriously, the number of cases may or may not be checked, so the criteria are unknown. I don't think the rule "Don't check the number of cases in front of for or foreach" will not fix it.

Also, people who handle exceptions with null write the same code even if it is not a collection, so this is also difficult to deal with. This exception handling problem is not unique to collections.

For the time being, "do not make the collection null" will solve the problem to some extent, so please practice it. However, this is not a uniform ban, and if you absolutely must make it null, you should explain it to the architect and get permission.

Sample code (Java) 2017/3/24 Added

    protected void main() {
        List<String> items = getDataFromDB();
        //NG1 NULL check before for
        if (items != null) {
            for (String item : items) {
                System.out.println(item);
            }
        }

        //NG2 NULL check and count check before for
        if (items != null && items.size() > 0) {
            for (String item : items) {
                System.out.println(item);
            }
        }

        //I'm assuming that NG3 NULL will not come, but for some reason check the number of cases
        if (items.size() > 0) {
            for (String item : items) {
                System.out.println(item);
            }
        }

        //Error if NG4 list is null
        if (items == null) {
            System.out.println("error");
        }
    }

    private List<String> getDataFromDB() {
        //Get data from DB and set NULL in case of error.
        //The new part is assumed to be getting data from Dao.
        try {

            List<String> list = new ArrayList<String>();
            list.add("1");
            list.add("2");
            return list;

        } catch (Exception e) {
            return null;
        }
    }

After refactoring (Java)

    protected void main() {
        Result result = getDataFromDB();
        List<String> items = result.getItems();
        //No null check or count check required
        for (String item : items) {
            System.out.println(item);
        }

        //If an exception has occurred, the exception information is output.
        if (result.getException() != null) {
            System.out.println(result.getException().toString());
        }
    }

    /**Get data from DB**/
    private Result getDataFromDB() {
        //Get data from DB and set NULL in case of error.
        //The new List part is assumed to be getting data from Dao.
        try {
            List<String> list = new ArrayList<String>();
            list.add("1");
            list.add("2");
            return new Result(list, null);

        } catch (Exception e) {
            return new Result(new ArrayList<String>(), e);
        }
    }

    /**Processing result class including return value and exception handling**/
    protected class Result {
        private List<String> items;
        private Exception exception;

        protected Result(List<String> items, Exception exception) {
            this.items = items;
            this.exception = exception;
        }

        protected List<String> getItems() {
            return items;
        }

        protected Exception getException() {
            return exception;
        }
    }

[Previous article (redundant assignment statement)] (http://qiita.com/csharpisthebest/items/f9ebc741e40037553d5b)

Next article (easy null check)

Table of Contents

Recommended Posts

Wasteful processing of collections-I want to give you a chance to write good code. 5 [C # refactoring sample]
Easy Null Check-I want to give you a chance to write good code. 6 [C # refactoring sample]
A little worrisome code collection-I want to give you a chance to write good code. 2 [C # refactoring sample]
Complicated conditional branching-I want to give you a chance to write good code. 1 [C # refactoring sample]
Bool type handling-I want to give you a chance to write good code. 7 [C # refactoring sample]
Too many function arguments-I want to give you a chance to write good code. 8 [C # refactoring sample]
A flowing interface? -I want to give you an opportunity to write good code. 3 [C # refactoring sample]
Is it possible to separate function calls and conditionals? --I want to give you a chance to write good code. 9 [C # refactoring sample]
Port C code with a lot of typecasts to Swift
How to write good code
A collection of patterns that you want to be aware of so as not to complicate the code
How to write when you want to handle "array of C language strings" like argv [] in Ruby-FFI
A memo when you want to clear the time part of the calendar
I want to write a nice build.gradle
Think of RxJava as a library that makes asynchronous processing easier to write
I made a sample of how to write delegate in SwiftUI 2.0 using MapKit