Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Addressbook::addTag() is not used #169 #179

Merged
merged 5 commits into from
Feb 20, 2017
Merged

Addressbook::addTag() is not used #169 #179

merged 5 commits into from
Feb 20, 2017

Commits on Feb 15, 2017

  1. AddressBook: remove unused addTag() 

    AddressBook#addTag() adds a Tag to the master list of Tags in
AddressBook.

This method is not used anywhere, and can be classified as unused code.
There are no commands that support the addition of Tags only, and it is
also not part of any learning objectives. Furthermore,
AddressBook#syncTagsWithMasterList() already handles the addition of
Tags from a Person gracefully without using exceptions. It should be
removed as unused code contributes to maintenance and comprehension
overheads.

Let's remove AddressBook#addTag().
    @PierceAndy
    PierceAndy committed Feb 15, 2017
    Configuration menu
    Copy the full SHA
    419ec2b View commit details
    Browse the repository at this point in the history

  2. UniqueTagList: remove unused add() 

    UniqueTagList#add() adds a Tag to the list of unique Tags, and throws an
exception if it is a duplicate of an existing Tag in the list.

With the removal of AddressBook#addTag(), this method is not used
anywhere, and can be classified as unused code. It should be removed as
unused code contributes to maintenance and comprehension overheads.

Let's remove UniqueTagList#add().
    @PierceAndy
    PierceAndy committed Feb 15, 2017
    Configuration menu
    Copy the full SHA
    84c8535 View commit details
    Browse the repository at this point in the history

  3. AddressBook: remove unused removeTag() 

    AddressBook#removeTag() removes a Tag from the master list of Tags and
from any Person in AddressBook.

This method is not used anywhere, and can be classified as unused code.
There are no commands that support the removal of Tags only, and it is
also not part of any learning objectives. It should be removed as unused
code contributes to maintenance and comprehension overheads.

Let's remove AddressBook#removeTag().
    @PierceAndy
    PierceAndy committed Feb 15, 2017
    Configuration menu
    Copy the full SHA
    fb8863d View commit details
    Browse the repository at this point in the history

  4. UniqueTagList: remove unused remove() 

    UniqueTagList#remove() removes a Tag from the list of unique Tags, and
throws an exception if it is not found in the list.

With the removal of AddressBook#removeTag(), this method is not used
anywhere, and can be classified as unused code. It should be removed as
unused code contributes to maintenance and comprehension overheads.

Let's remove UniqueTagList#remove().
    @PierceAndy
    PierceAndy committed Feb 15, 2017
    Configuration menu
    Copy the full SHA
    4e3fac9 View commit details
    Browse the repository at this point in the history

  5. AddressBook: remove unused containsTag() 

    AddressBook#containsTag() checks if the master list of Tags contains a
Tag with the same value as the given Tag.

AddressBook#containsTag() is not used in the production code. While it
is used in test code, its use can be replaced by
AddressBookTest#isTagObjectInAddressBookList().

Let's remove AddressBook#containsTag().

Notes:
* Why not keep AddressBook#containsTag() and remove
AddressBookTest#isTagObjectInAddressBookList() instead? The former does
a value equality test while the latter goes further and does a reference
equality test. The test code requires a reference equality test to
verify Person objects refer to Tag objects in the common tag list
instead of keeping its own copies of Tag objects.
* Why not change AddressBook#containsTag() to use reference equality and
use that in tests? Doing so will make AddressBook#containsTag()'s
semantics inconsistent with the rest of our API.
    @PierceAndy
    PierceAndy committed Feb 15, 2017
    Configuration menu
    Copy the full SHA
    a6cc952 View commit details
    Browse the repository at this point in the history