Skip to content

algorithms/conversion: convertBinaryToDecimal accepts non-binary input & crashes on sign; convertDecimalToBinary(0) returns empty string #56

@aimasteracc

Description

@aimasteracc

Description

The base-conversion algorithms under algorithms/conversion/ have three correctness bugs (all verified with swiftc).

1. convertBinaryToDecimal accepts non-binary digits (should return nil)

The validation only checks that the string parses as a base-10 Int, not that it contains only binary digits:

if let _ = Int(binary) {   // "2", "9", "123" all pass this check
    let digits = binary.map { Int(String($0))! }.reversed()
    ...
}

The docstring says "If it's not valid binary number … it returns nil", but:

convertBinaryToDecimal("2") = Optional(2)   // expected nil
convertBinaryToDecimal("9") = Optional(9)   // expected nil
convertBinaryToDecimal("101") = Optional(5) // correct

Digits 29 are treated as binary and produce a meaningless decimal value.

2. convertBinaryToDecimal crashes on a leading sign

Int("-101") succeeds (it is a valid base-10 int), so the guard passes; then "-101".map { Int(String($0))! } force-unwraps Int("-") which is nil:

convertBinaryToDecimal("-101")
// Fatal error: Unexpectedly found nil while unwrapping an Optional value

The function returns Int? (designed to signal bad input with nil) but instead crashes on it.

3. convertDecimalToBinary(0) returns "" instead of "0"

while decimal != 0 { ... }   // never executes for decimal == 0
convertDecimalToBinary(0) = ""   // expected "0"

0 is a non-negative number (which the docstring says it accepts), and its binary form is "0", not the empty string.

Expected behavior

  • convertBinaryToDecimal returns nil for any string containing a non-binary character (including 29 and signs), never crashes.
  • convertDecimalToBinary(0) returns "0".

Actual behavior

  • convertBinaryToDecimal("2")2, ("9")9 (non-binary accepted); ("-101") → crash.
  • convertDecimalToBinary(0)"".

Suggested fix

  • Validate against binary digits, e.g. guard binary.allSatisfy({ $0 == "0" || $0 == "1" }) else { return nil }, and avoid the force-unwrap.
  • In convertDecimalToBinary, special-case 0 (if decimal == 0 { return "0" }) — and consider handling/ rejecting negatives, since the loop misbehaves for them too.

Also: convertBinaryToDecimal contains a leftover print(digits) debug statement.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions