vkostyukov / la4j

Linear Algebra for Java
http://la4j.org
Apache License 2.0
373 stars 109 forks source link

202 range checks #218

Closed DWiechert closed 9 years ago

DWiechert commented 9 years ago

Sending this before doing all of the Matrix range checking to see if this is on the right track with what you had in mind. I tested the individual ensureIndex method but did not add tests for each method on CompressedVector that calls it. If you would like to me add those tests I can.

vkostyukov commented 9 years ago

I had a quick look at Vector interface. I think we can start with adding this ensureIndexInBounds check in just two methods get and set. All other methods depends on them, so we will cover everything.

DWiechert commented 9 years ago

Sure, I'll make these changes and update.

vkostyukov commented 9 years ago

Great! Thanks! Please, go ahead with writing the same checks for Basic1DMatrix, CRSMatrix and CCSMatrix classes.

vkostyukov commented 9 years ago

Oh. Looks like once we migrated to JUunit 4 we broke this PR:

[ERROR] /home/travis/build/vkostyukov/la4j/src/test/java/org/la4j/vector/sparse/SparseVectorTest.java:[131,9] error: cannot find symbol
[ERROR]   symbol:   method fail(String)
  location: class SparseVectorTest. 
vkostyukov commented 9 years ago

@DWiechert, could you please fix and send another PR. It's just a couple of new tests you wrote that uses fail method, which is not available in JUnit 4.

DWiechert commented 9 years ago

Yes. I'll get the latest changes and merge them into my branch and send another PR.