Originally Posted by
Minike
I ported it to Scala 1 on 1 for now so I have the possibility of changing it later.
I am fully aware Scala has interoperability with Java, but like I said, this way I would lose the freedom of directly editing it as Scala.
The library is 100% feature-complete, seeing as we invented the format. I don't see a particular reason why you'd bother copy-pasteing all of my code and then updating the syntax. Also, please remove attribution to me on this code that I did not write, and change the name of the library in your project. It's a derivative work. I'm not responsible for it. It is not in any way associated with me.
Originally Posted by
Minike
The tone in your post is set completely wrong, so I would like to ask you to other be helpful or not reply at all.
Thank you, tone police.
Originally Posted by
Minike
As like I mentioned, I have no previous experience with Scala, so you are just behaving like a complete ass to somebody who is still learning.
I'm behaving like an ass because I said that you wrote this in Java style? Ok then.
Originally Posted by
Minike
But if you happen to have any good tips or hints, please feel free to post those.
You use Java-style inheritance like so:
Code:
class NXLongNode(override val name: String,
override val file: NXFile,
override val childIndex: Int,
override val childCount: Int,
val reader: BufferedReader)
extends NXNode(name, file, childIndex, childCount) {
final val value = reader.getLong
override def get(): Long = value
}
Use a trait like so:
Code:
sealed trait NXNode(name: String, file: NXFile, childIndex: Int, childCount: Int)
case class NXLongNode(name: String, file: NXFile, childIndex: Int, childCount: Int, value: Long) extends NXNode(name, file, childIndex, childCount)
...
You allow things to be null:
Code:
class NXNullNode(override val name: String,
override val file: NXFile,
override val childIndex: Int,
override val childCount: Int,
val reader: BufferedReader)
extends NXNode(name, file, childIndex, childCount) {
reader.skip(8)
override def get(): Any = null
}
Use Unit (the only value of type Unit is ()) in this case.
Code:
def resolve(path: Array[String]): NXNode = {
var cursor: NXNode = getRoot
for (piece <- path) {
if (cursor == null)
return null
cursor = cursor.getChild(piece)
}
cursor
}
Use Option!
On that note, you also use Java-style iterators and for loops.
Code:
for (node <- reader.resolve("MakeCharInfo.img/Info")) {
for (subNode <- node) {
// CharFemale | CharMale
for (valueNode <- subNode) {
val value = valueNode.asInstanceOf[NXLongNode].get()
if (value > 1e6)
allowedEquips += value
}
}
}
Use maps and folds!
Also, if you use traits, you won't have to write garbage like this:
Code:
valueNode.asInstanceOf[NXLongNode].get()
You also have mutability everywhere. The whole thing should be stateless. You don't need state at all for this task!
EDIT: adding more things
You keep writing unnecessary things like keywords for constructor parameters.
Code:
class LazyNXFile(val buffer: ByteBuffer)
can just be
Code:
class LazyNXFile(buffer: ByteBuffer)
You have an empty Iterator.
Code:
final class EmptyNodeIterator
extends Iterator[NXNode] {
override def hasNext: Boolean = false
override def next(): NXNode = null
}
This is normal crufty Java boilerplate, but if you used Seq, you wouldn't need this.
Reply With Quote
