Skip to content

Code Review #3

@anselb

Description

@anselb

I think your library is pretty good at the moment. Don't feel like you need to change anything I mention about your code.

Readability and Formatting

  • While I think upperWiggle is a good name and gets the main idea of the function across, I used the name evenCaps to indicate that it is the even indexed characters that get uppercased.
  • I would like to see some comments in your problems file. For example, what is the regex inside of .replace(/\s+/g, '-') doing?
  • Your variable names and indentation are mostly readable besides what I have mentioned.

Organization and Modularity

  • While some of your methods rely on other methods, I think that is okay. Since these methods are bundled together as a library, I don't think someone would need to worry about a method going away, or changing implementation, and breaking the other methods.
  • It seems all of your functions avoid side effects; they seem to all return a new, modified string, rather than modifying the original string.

Standard Library/Conventions

  • Nice usage of the standard library methods. I think it makes the code easier to read than reimplementing it again.
  • While the assignment does call for methods to be created on the string prototype, I think it would be nice to have another file that contains just the functions by themselves. That way, developers have the option of using the extended String methods or importing the independent functions.

Effectiveness of Solution

  • I would say you met all requirements of the assignment. It would have been nice to see oddCaps(), but I think you made the work up by implementing other methods.
  • Because your solution uses a lot of built-ins, I would guess your solutions handle almost all, if not all, edge cases. For example, using .charAt() is great because if that char index does not exist, it will not break.

Testing and Error Handling

  • You're tests seem to cover most, if not all, typical input cases, and some edge cases.
  • I think you could have included some more edge cases in your tests to show that they work, such an empty string, or a string with unusual characters (é or ñ).

Algorithmic Complexity

  • I liked how you called .toUpperCase() and .toLowerCase() on every other character. I believe I just called .toLowerCase() on everything, and then, I called .toUpperCase() on every other character, which is twice as slow.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions