Excellent. Thanks for the review Schubert. I'd suggest you collect all your comments into one JIRA (Below all looks good except for perhaps the last two suggestions). Thanks, St.Ack
On Sun, Aug 30, 2009 at 3:33 AM, Schubert Zhang <[email protected]> wrote: > Hi stack, > > I am reviewing the code of HFile. And I will post my comments here. > > Comments: > > 1. a minor mistake in HFile.Writer.writeFileInfo(FSDataOutputStream o) > > int avgValueLen = this.entryCount == 0? 0: > (int)(this.keylength/this.entryCount); > > Here this.keylength should be this.valuelength. > > 2. I found it is very uncomfortable to check or convert data types of > Integer and Long. > I think HFile will be a common file format in the future, so it is > better to avoid limitations. > > My suggestions: > > private int totalBytes = 0; // long is better, in fact the parameter in > Trailer is long. > private int entryCount = 0; // long is better > private int blocksize; //maybe long is better? > > it we change type of these variables, some parameters in some methods > should also be changed. > > Schubert >
