Refactored makeRoomForBlock.

vmware-archive / quickstep

Quickstep Project

Apache License 2.0

27 stars 13 forks source link

Refactored makeRoomForBlock. #192

Closed zuyu closed 8 years ago

zuyu commented 8 years ago

This refactor includes:

Renamed to makeRoomForBlockOrBlob.
Removed a unnecessary variable.
Simplified the if-else statement.
Added more comments about this method.

cramja commented 8 years ago

@zuyu did we run the TPCH query 8/9 on this?

cramja commented 8 years ago

@zuyu Also, when thinking about refactoring all the method names in EvictionPolicy.hpp from blockPastTenseVerb to blockOrBlobPastTenseVerb I think it seems too wordy.

Maybe we could make the naming more succinct to instead name them all of the form notifyPastTenseVerb(block_id block_or_blob).

What do you think?

cramja commented 8 years ago

BTW pastTenseVerb is one of {created, evicted, unreferenced, referenced, deleted}

zuyu commented 8 years ago

@cramja I did not run TPC-H over this PR. This just refactors a method internally.

zuyu commented 8 years ago

@cramja Alternatively for EvictionPolicy, we could add a comment before the class definition: block refers to a block or blob.

cramja commented 8 years ago

@zuyu I think the simplest thing here is to use you approach. I will make a PR for this.

pateljm commented 8 years ago

Merging.