3
\$\begingroup\$

My function takes a string of Name and returns First Name, Last Name and Middle Name as output.

Please review and help me optimize it.

    //input --> Hamza Khan
    ProcessFullName(full_name): Object {
        var Obj = {
            FirstName: "",
            LastName: "",
            MiddleName: ""
        };
        var splitName = full_name.split(" ");
        if (splitName.length>0) {
            switch (splitName.length) {
                case 1:
                    Obj.FirstName = splitName[0];
                    break;
                case 2: 
                    Obj.FirstName = splitName[0];
                    Obj.LastName = splitName[1];
                    break;
                case 3: 
                    Obj.FirstName = splitName[0];
                    Obj.MiddleName = splitName[1];
                    Obj.LastName = splitName[2]; 
                    break;
            }
        }
        return Obj;
    } //Output --> { FirstName: "Hamza", LastName: "Khan", MiddleName: "" }
\$\endgroup\$
3
  • \$\begingroup\$ The assignments and break statements per case are more verbose than returning an object literal. In turn that would remove the temporary mutable variable. \$\endgroup\$ – Aluan Haddad Mar 12 '18 at 9:38
  • 4
    \$\begingroup\$ As a side note, you might want to be careful about making the assumption that all names work the same way. What if a person has two first names that are split with a space, or two last names? Good reading: kalzumeus.com/2010/06/17/… \$\endgroup\$ – Hannele Mar 12 '18 at 13:43
  • \$\begingroup\$ It may be useful to provide information about where you get the name from. Is it user input (you can let people fill in a form with firstName, lastName and MiddleName) , or is it generated by yourself (then you can take it from your own database and split it up it there)? \$\endgroup\$ – Rob Monhemius Mar 13 '18 at 16:13
2
\$\begingroup\$
  • Provide a type for your full_name argument and a more specific type than Object for the return type:

    interface SplitNameData {  // Does anybody have a better interface name?
      FirstName: string;
      MiddleName: string;
      LastName: string;
    }
    
    ProcessFullName(full_name: string): SplitNameData {
      const Obj: SplitNameData = {
        firstName: '',
        middleName: '',
        lastName: ''
      };
    
  • I think it would be cleaner to use null as a marker for non-existing names instead of the empty string even though the empty is probably not a valid name anywhere in the world.

  • What is your function supposed to do with names having more than three parts?

  • Consider using const wherever possible and let in all other cases. I recommend to never use var. ⇒ const Obj, const splitName.

  • Use a more descriptive name for Obj, e.g. splitNameData or separatedNames. In this case, I would recommend to rename splitName to something like nameParts as well to avoid confusion.

  • Use a consistent naming scheme: fullName instead of full_name, firstName instead of FirstName and so on.

  • Use array destructuring:

    case 2:
      [Obj.FirstName, Obj.LastName] = splitName;
      break;
    case 3:
      [Obj.FirstName, Obj.MiddleName, Obj.LastName] = splitName;
      break;
    
  • ad performance: for this computationally simple task, any optimization would be premature optimization in my opinion.

  • Alternative (maybe cleaner?) solution using named RegExp capture groups:

    const NAME_PATTERN = /^(?<firstName>[^ ]*) ((?<middleName>[^ ]*) )?(?<lastName>[^ ]*)$/;
    
    const emptyNameData = {
      firstName: '', middleName: '', lastName: ''
    };
    
    const {
      groups: {
        firstName,
        middleName = '',
        lastName
      } = emptyNameData
    } = NAME_PATTERN.exec(fullName) || {};
    alert([firstName, middleName, lastName]);
    

    I am not very keen on the || {} part, maybe an if-else checking the match object for null would be a better solution.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks. Up voted! How can I handle the corner cases of big names? For eg. What if the name is greater than 4 words? \$\endgroup\$ – hamzox Mar 12 '18 at 9:10
  • 1
    \$\begingroup\$ @hamzox That depends on what the function is supposed to do in that case. What is your actual use case for splitting the name? There are too many possibilities around the globe (e.g. see here) to cover them all. The same holds true for mail addresses. \$\endgroup\$ – ComFreek Mar 12 '18 at 9:18
  • \$\begingroup\$ Yeah there are too many possibilities. I think I am good with the solution for now. Thanks once again. \$\endgroup\$ – hamzox Mar 12 '18 at 9:45
1
\$\begingroup\$

I will assume that the argument full_name (which would be better named fullName) is always a string as that is what your code implies.

Also assuming that undefined names are to be set to "" (empty string) and that all middle names are to be concatenated to a single string separated with spaces.

Rename properties removing the "name" part as that is implied by what the object is holding.

You don't need to store the result object, just return it directly.

The resulting function

ProcessFullName(fullName): Object { 
    const names = fullName.split(" ");
    const vet = name => name ? name : "";
    return {
        first:  vet(names.shift()),
        last:   vet(names.pop()),
        middle: vet(names.join(" "))
    };
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.