Skip to content

Optimize space#16

Merged
tonyarnold merged 1 commit into
tonyarnold:masterfrom
Damonvvong:master
Mar 7, 2018
Merged

Optimize space#16
tonyarnold merged 1 commit into
tonyarnold:masterfrom
Damonvvong:master

Conversation

@Damonvvong
Copy link
Copy Markdown

No description provided.

@tonyarnold
Copy link
Copy Markdown
Owner

Interesting! Could you give me a really brief description of what your changes do?

@Damonvvong
Copy link
Copy Markdown
Author

Damonvvong commented Feb 5, 2018

First of all,the k = x - y, so k is in [-M,N]
so I change the length of vertices to [0...M+N], it is -M...N in the whitepaper

reference

// before
var vertices = Array(repeating: -1, count: 2 * Int(max) + 1) // from [0...2*max], it is -max...max in the whitepaper
// after
var vertices = Array(repeating: -1, count: max + 1) // from [0...N+M], it is -M...N in the whitepaper

And then, I thing we should skip when the k is not in [-M,N]

guard k >= -toCount && k <= fromCount else {
    continue
}

last,I think force unwraps is not the good choose,so i change the ! to ??
reference

if traceType == .insertion {
    let x = traceStep.nextX!
    let x = traceStep.nextX ?? -1
    return Trace(from: Point(x: x, y: x - k - 1), to: Point(x: x, y: x - k), D: D)		              
} else {
    let x = traceStep.previousX! + 1
    let x = (traceStep.previousX ?? 0) + 1
    return Trace(from: Point(x: x - 1, y: x - k), to: Point(x: x, y: x - k), D: D)
}

@tonyarnold

@tonyarnold
Copy link
Copy Markdown
Owner

Thanks for the run down @Damonvvong - I was referring to the impact upon memory use, performance, etc. I should have been more specific in my language.

@Damonvvong
Copy link
Copy Markdown
Author

Sorry, I don't konw what you want.
So, I close it ~

@Damonvvong Damonvvong closed this Feb 10, 2018
@tonyarnold
Copy link
Copy Markdown
Owner

Please accept my apologies, @Damonvvong - I was only interested in what functional changes there might be in performance and behaviour after merging your changes. I have not had time to properly review and run tests against what you've contributed yet.

@tonyarnold tonyarnold reopened this Feb 11, 2018
@tonyarnold tonyarnold merged commit 94b2725 into tonyarnold:master Mar 7, 2018
@tonyarnold tonyarnold mentioned this pull request Apr 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants