HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Alternate form of BufferedInputStream

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bufferedinputstreamformalternate

Problem

I was having a problem with the TCP/IP socket communications in a web app where the server that the app was talking to would occasionally send bursts of data that would overflow the low level stream buffer, resulting in lost data.

The solution I came up with is basically an unthreaded form of the StreamGobbler and I have been calling it a GreedyBufferedInputStream. The basic idea is that just about any call to this class will include draining the source InputStream.

I'm still fairly new to Scala, so please let me know how this code could be improved. Also, is there anything in the code which could be done more efficiently? Performance is critically important.

```
package edu.stsci.util

import org.slf4j.Logger

import java.io.InputStream
import java.util

class GreedyBufferedInputStream extends InputStream {
private var logger: Logger = null
private var source: InputStream = null
private val data: util.LinkedList[DataBlock] = new util.LinkedList[DataBlock]()
private var currentBlock: DataBlock = null

def this(logger: Logger, source: InputStream) {
this()

this.logger = logger
this.source = source

drainSource()
}

def this(source: InputStream) {
this(null, source)
}

def read() = {
prepareToRead()
if (currentBlock == null) -1
else currentBlock.read()
}

override def read(destination: Array[Byte], offset: Int, length: Int): Int = {
prepareToRead()
if (currentBlock == null) return -1

var bytesRead = currentBlock.read(offset, length, destination)

while (bytesRead 0) {
val raw = new ArrayByte)
val length = source.read(raw)
val block = new DataBlock(raw, length)
data.add(block)
}
}

private def prepareToRead() {
drainSource()

if (currentBlock != null) {
val done = currentBlock.isDone
if (done) currentBlock = null
else return // we have a current block
}

if (data.isEmpty) { // no choice but to block

Solution

Just some random bits of feedback, not really addressing the code as a whole.

First, I would handle constructing the class as follows:

class GreedyBufferedInputStream(logger: Logger = null, initialSource: InputStream)
extends InputStream {

  private var source: InputStream = initialSource
  private val data: util.LinkedList[DataBlock] = new util.LinkedList()
  private var currentBlock: DataBlock = null

  drainSource()


This eliminates a var and both auxiliary constructors.

I would rewrite the available() method as follows:

override def available() = {
    drainSource()
    import collection.JavaConverters._
    data.asScala.map(_.available).sum +
      Option(currentBlock).map(_.available).getOrElse(0)
  }


And I would replace this:

var readCount = {
  if (available >= length) length
  else available
}


with

val readCount = length min available

Code Snippets

class GreedyBufferedInputStream(logger: Logger = null, initialSource: InputStream)
extends InputStream {

  private var source: InputStream = initialSource
  private val data: util.LinkedList[DataBlock] = new util.LinkedList()
  private var currentBlock: DataBlock = null

  drainSource()
override def available() = {
    drainSource()
    import collection.JavaConverters._
    data.asScala.map(_.available).sum +
      Option(currentBlock).map(_.available).getOrElse(0)
  }
var readCount = {
  if (available >= length) length
  else available
}
val readCount = length min available

Context

StackExchange Code Review Q#13536, answer score: 3

Revisions (0)

No revisions yet.