Redefining == operator doesn't behave expectedly

New to Julia
1

I have a problem with a coding execise from Exercism, i habe been trying to solve it for some hours now

I think the problem lies with the connectd == and != as if i tests the statements individually they work out.
Does anyone know how to redefine the == so that the test will go through

The code doesnt clear one of the tests:

function toIntArray(ISBN::String)
    dig ="0123456789"
    isbn=[]
    for i in 1:length(ISBN)
        if in(ISBN[i],dig)
            append!(isbn,Int(ISBN[i]-48))
            elseif  ISBN[i]=='X' && (i-length(ISBN))==0
            append!(isbn,10)
            elseif ISBN[i] =='-'
            else
                throw(DomainError("Snake on the plane"))
        end
    end
    return(isbn)
end


struct ISBN
    isbn::Array{Int}

    function ISBN(ISBN::String)
        storage=0
        isbn = toIntArray(ISBN)
        if length(isbn)!= 10
            throw(DomainError("tooloongortooshoort"))
        end

        for i in 1:length(isbn)
            storage=storage+ isbn[i]*(11-i)
        end
        if storage %11 ==0 
            return(true) 
        else 
            throw(DomainError("Thats the wrong number"))
        end
    end
end

function Base.:(==)(i1::ISBN, i2::ISBN)
    return toIntArray(i1)==toIntArray(i2)
end
function Base.:(!=)(i1::ISBN, i2::ISBN)
    return toIntArray(i1)!=toIntArray(i2)
end 
´´´ 
The test is 
´´´ ISBN("3-598-21508-8") == ISBN("3598215088") != ISBN("3-598-21507-X") ```
(should result in true but results in false)



the ecercise can be found at https://exercism.org/tracks/julia/exercises/isbn-verifier
1 Like
2

Welcome to the forum! :slight_smile:

I don’t want to solve the exercise for you so I will just give you some hints:

  1. You should check what ISBN("3-598-21508-8") returns. It is not of type ISBN because your constructor (the ISBN function inside the struct’s definition) never calls new to actually construct an instance. Have a look at the manual’s section on constructors
  2. Once you fix that, you will notice that your definitions of == and != don’t make sense as you didn’t define a method for toIntArray that takes ISBN types. So you’ll probably want to rethink what these methods should do.

Come back if you need more hints!

Edit: When you solved it, you can also post your solution and I will give you some pointers how to make it more idiomatic and point out some beginner mistakes i.e. concerning the typing of variables :slight_smile:

11 Likes
3

Hey abraemer,

Great advice
i have succceeded to resolve the problem with your help i think its not the most elegant solution but it works
I changes the constructor from returning the ISBN to new(isbn) # I think that leads to a new instance being created ?
I overloaded the toIntArray function to facilitate it to turn instances of isbn into an IntArray

Thank you very much vor your help! :grinning:

Here is the current code that manages to pass all the tests:

struct ISBN
    isbn::Array{Int}

    function ISBN(ISBN::String)
        storage=0
        isbn = toIntArray(ISBN)
        if length(isbn)!= 10
            throw(DomainError("tooloongortooshoort"))
        end

        for i in 1:length(isbn)
            storage=storage+ isbn[i]*(11-i)
        end
        if storage %11 ==0 
            new(isbn) 
        else 
            throw(DomainError("Thats the wrong number"))
        end
    end
end

function toIntArray(ISBN::String)
    dig ="0123456789"
    isbn=[]
    for i in 1:length(ISBN)
        if in(ISBN[i],dig)
            append!(isbn,Int(ISBN[i]-48))
            elseif  ISBN[i]=='X' && (i-length(ISBN))==0
            append!(isbn,10)
            elseif ISBN[i] =='-'
            else
                throw(DomainError("Snake on the plane"))
        end
    end
    return(isbn)
end

function toIntArray(isbn::ISBN)
    dig ="0123456789"
    result=[]
    for i in 1:10
        append!(result, Int(isbn.isbn[i]))
    end
    return(result)
end

function Base.:(==)(i1::ISBN, i2::ISBN)
    return toIntArray(i1)==toIntArray(i2)
end
4

Glad my hints where helpful :slight_smile:

As promised some comments:

  • General logic: your ISBN struct stores the digits of the ISBN, so when you want to compare to instances, you could just compare their arrays directly. Your function toIntArray(isbn::ISBN) really just performs a slow copy of the stored array. So you can just do:
Base.:(==)(i1::ISBN, i2::ISBN) = i1.isbn == i2.isbn
  • Typing: You annotated the field isbn in your ISBN struct with Array{Int} which is not a good annotation because Array denotes a multi-dimensional array and its full type signature carrys the dimension as well. So it would be better (for performance) to write the type as Array{Int, 1} or just Vector{Int} which means the same thing. However you could also just leave the type out if you don’t care about performance. Julia does not require you to give fields a type.
  • You could shorten the toIntArray function a bit using Julia’s isdigit function and a bit of broadcasting magic (see manual):
function toIntArray(ISBN::String)
    digits_string = filter(isdigit, ISBN)
    digits = parse.(Int, collect(digits_string)) # note that strings are scalars for broadcasting, so I use collect to turn it into a Vector{Char}
    if ISBN[end] == 'X'
        push!(digits, 10)
    end
    return digits
end
  • you could consider making the verification of an ISBN its own function. That way you could use that functionality elsewhere and it also would clear up the constructor a bit.
6 Likes
5

Thanks for the great feedback! I‘ll try to implement it! :grinning: