Skip to main content
added 80 characters in body
Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65
  1. It looks like you either imported std::string or the whole standard namespace into the global namespace.
    It's ok to do the former in implementation-files, though I would desist as it doesn't gain you all that much in brevity.
    If it's the latter, read "Why is “using namespace std;” considered bad practice?" and change it.

  2. Avoid allocating memory. Doing so is slow and can fail. That means accepting the arguments by constant reference, not using temporary copies, and not modifying the arguments.
    If you had C++17, it would mean changing to std::string_view for the added flexibility.

  3. Prefer using auto to a more complicated expression using decltype. It's less error-prone, more readable, and also shorter.

  4. <algorithm> contains std::min(). Using that is more readable, shorter and no less efficient than writing it out using the conditional-operator.

  5. Did you test ("abc", "b")? That's two deletions edit-distance, but will be accepted anyway.

  6. Keep your line-length reasonable. Horizontal scrolling kills readability.

  1. It looks like you either imported std::string or the whole standard namespace into the global namespace.
    It's ok to do the former in implementation-files, though I would desist as it doesn't gain you all that much in brevity.
    If it's the latter, read "Why is “using namespace std;” considered bad practice?" and change it.

  2. Avoid allocating memory. Doing so is slow and can fail. That means accepting the arguments by constant reference, not using temporary copies, and not modifying the arguments.
    If you had C++17, it would mean changing to std::string_view for the added flexibility.

  3. Prefer using auto to a more complicated expression using decltype. It's less error-prone, more readable, and also shorter.

  4. <algorithm> contains std::min(). Using that is more readable, shorter and no less efficient than writing it out using the conditional-operator.

  5. Did you test ("abc", "b")? That's two deletions edit-distance, but will be accepted anyway.

  1. It looks like you either imported std::string or the whole standard namespace into the global namespace.
    It's ok to do the former in implementation-files, though I would desist as it doesn't gain you all that much in brevity.
    If it's the latter, read "Why is “using namespace std;” considered bad practice?" and change it.

  2. Avoid allocating memory. Doing so is slow and can fail. That means accepting the arguments by constant reference, not using temporary copies, and not modifying the arguments.
    If you had C++17, it would mean changing to std::string_view for the added flexibility.

  3. Prefer using auto to a more complicated expression using decltype. It's less error-prone, more readable, and also shorter.

  4. <algorithm> contains std::min(). Using that is more readable, shorter and no less efficient than writing it out using the conditional-operator.

  5. Did you test ("abc", "b")? That's two deletions edit-distance, but will be accepted anyway.

  6. Keep your line-length reasonable. Horizontal scrolling kills readability.

edited body
Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65
#include "one_away.h"
#include <iostream>
#include <cstdlib>

static bool do_test(const std::string& a, const std::string& b, bool expected) {
    bool r = oneAway(a, b);
    std::cout << (r == expected ? "[OK] " : "[FAIL] ")
        << std::boolalpha << r << " \"" << a << "\" \"" << b << \"\n";
    return r == expected;
}

static voidbool test(const std::string& a, const std::string& b = a, bool expected = true) {
    bool r = do_test(a, b, expected);
    if (a != b)
        r &= do_test(b, a, expected);
    return r;
}

int main() {
    bool r = test("");
    r &= test("abc");
    r &= test("pale", "ple");
    r &= test("pales", "pale");
    r &= test("pale", "bale");
    r &= test("pale", "bake", false);
    r &= test("abc", "b", false);
    std::cout << (r ? "[OK]\n" : "[FAIL]\n");
    return r ? 0 : EXIT_FAILURE;
}
#include "one_away.h"
#include <iostream>
#include <cstdlib>

static bool do_test(const std::string& a, const std::string& b, bool expected) {
    bool r = oneAway(a, b);
    std::cout << (r == expected ? "[OK] " : "[FAIL] ")
        << std::boolalpha << r << " \"" << a << "\" \"" << b << \"\n";
    return r == expected;
}

static void test(const std::string& a, const std::string& b = a, bool expected = true) {
    bool r = do_test(a, b, expected);
    if (a != b)
        r &= do_test(b, a, expected);
    return r;
}

int main() {
    bool r = test("");
    r &= test("abc");
    r &= test("pale", "ple");
    r &= test("pales", "pale");
    r &= test("pale", "bale");
    r &= test("pale", "bake", false);
    r &= test("abc", "b", false);
    std::cout << (r ? "[OK]\n" : "[FAIL]\n");
    return r ? 0 : EXIT_FAILURE;
}
#include "one_away.h"
#include <iostream>
#include <cstdlib>

static bool do_test(const std::string& a, const std::string& b, bool expected) {
    bool r = oneAway(a, b);
    std::cout << (r == expected ? "[OK] " : "[FAIL] ")
        << std::boolalpha << r << " \"" << a << "\" \"" << b << \"\n";
    return r == expected;
}

static bool test(const std::string& a, const std::string& b = a, bool expected = true) {
    bool r = do_test(a, b, expected);
    if (a != b)
        r &= do_test(b, a, expected);
    return r;
}

int main() {
    bool r = test("");
    r &= test("abc");
    r &= test("pale", "ple");
    r &= test("pales", "pale");
    r &= test("pale", "bale");
    r &= test("pale", "bake", false);
    r &= test("abc", "b", false);
    std::cout << (r ? "[OK]\n" : "[FAIL]\n");
    return r ? 0 : EXIT_FAILURE;
}
Added improved code
Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65

Using all that, but staying true to C++11:

#include <string>
#include <algorithm>

bool oneAway(const std::string& a, const std::string& b) noexcept {
    if (a.size() > b.size())
        return oneAway(b, a);
    if (b.size() - a.size() > 1) // No need to look further, pure optimization
        return false;
    auto begin = std::mismatch(a.begin(), a.end(), b.begin());
    using reverse_it = decltype(a.rbegin()); // std::make_reverse_iterator is C++14
    auto end = std::mismatch(a.rbegin(), reverse_it(begin.first), b.rbegin());
    return end.second.base() - begin.second < 2;
}

And for testing:

#include "one_away.h"
#include <iostream>
#include <cstdlib>

static bool do_test(const std::string& a, const std::string& b, bool expected) {
    bool r = oneAway(a, b);
    std::cout << (r == expected ? "[OK] " : "[FAIL] ")
        << std::boolalpha << r << " \"" << a << "\" \"" << b << \"\n";
    return r == expected;
}

static void test(const std::string& a, const std::string& b = a, bool expected = true) {
    bool r = do_test(a, b, expected);
    if (a != b)
        r &= do_test(b, a, expected);
    return r;
}

int main() {
    bool r = test("");
    r &= test("abc");
    r &= test("pale", "ple");
    r &= test("pales", "pale");
    r &= test("pale", "bale");
    r &= test("pale", "bake", false);
    r &= test("abc", "b", false);
    std::cout << (r ? "[OK]\n" : "[FAIL]\n");
    return r ? 0 : EXIT_FAILURE;
}

Using all that, but staying true to C++11:

#include <string>
#include <algorithm>

bool oneAway(const std::string& a, const std::string& b) noexcept {
    if (a.size() > b.size())
        return oneAway(b, a);
    if (b.size() - a.size() > 1) // No need to look further, pure optimization
        return false;
    auto begin = std::mismatch(a.begin(), a.end(), b.begin());
    using reverse_it = decltype(a.rbegin()); // std::make_reverse_iterator is C++14
    auto end = std::mismatch(a.rbegin(), reverse_it(begin.first), b.rbegin());
    return end.second.base() - begin.second < 2;
}

And for testing:

#include "one_away.h"
#include <iostream>
#include <cstdlib>

static bool do_test(const std::string& a, const std::string& b, bool expected) {
    bool r = oneAway(a, b);
    std::cout << (r == expected ? "[OK] " : "[FAIL] ")
        << std::boolalpha << r << " \"" << a << "\" \"" << b << \"\n";
    return r == expected;
}

static void test(const std::string& a, const std::string& b = a, bool expected = true) {
    bool r = do_test(a, b, expected);
    if (a != b)
        r &= do_test(b, a, expected);
    return r;
}

int main() {
    bool r = test("");
    r &= test("abc");
    r &= test("pale", "ple");
    r &= test("pales", "pale");
    r &= test("pale", "bale");
    r &= test("pale", "bake", false);
    r &= test("abc", "b", false);
    std::cout << (r ? "[OK]\n" : "[FAIL]\n");
    return r ? 0 : EXIT_FAILURE;
}
edited body
Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65
Loading
Source Link
Deduplicator
  • 19.3k
  • 1
  • 31
  • 65
Loading