Skip to main content
added 239 characters in body
Source Link
Jerry Coffin
  • 33.6k
  • 4
  • 75
  • 143

I see a few points that seem open to improvement.

First of all, in some cases you document the "contract" based on the type of the input and output ("number -> number") but in others the unit of measure ("Miles -> furlongs").

The units of measure are already documented directly in the function's name, so the type is the only one that really makes much sense to document in comments. For that, you'd probably be better off with a single header documenting the contract for all the functions.

Second, essentially all of these are really just multiplying/dividing the input by a factor, and returning the result. As such, it would probably make more sense to pass the units to convert as parameters. Then you'd have a single function that looked up the correct factor from a table and returned the result based on that.

Third, I'd rather see a function that actually compared results and told me whether the test passed or failed. As it stands now, I'm left to scan through the results myself to see whether the functions worked correctly or not.

Finally, the units you've chosen to support seem rather strange, at least to me. "rods" are used almost exclusively in surveying, and furlongs are used almost exclusively in stories about medieval times. At the same time, although the initial discussion is about converting between Imperial and metric measures, the only metric unit you seem to support is the centimeter, not such common measures as meters or kilometers.

P.S. The basic premise in the exercise is also somewhat incorrect: not all the rest of the world uses the metric system. For example, Imperial measures are still used quite a bit in the UK, and in some other former UK colonies as well.

I see a few points that seem open to improvement.

First of all, in some cases you document the "contract" based on the type of the input and output ("number -> number") but in others the unit of measure ("Miles -> furlongs").

The units of measure are already documented directly in the function's name, so the type is the only one that really makes much sense to document in comments. For that, you'd probably be better off with a single header documenting the contract for all the functions.

Second, essentially all of these are really just multiplying/dividing the input by a factor, and returning the result. As such, it would probably make more sense to pass the units to convert as parameters. Then you'd have a single function that looked up the correct factor from a table and returned the result based on that.

Third, I'd rather see a function that actually compared results and told me whether the test passed or failed. As it stands now, I'm left to scan through the results myself to see whether the functions worked correctly or not.

Finally, the units you've chosen to support seem rather strange, at least to me. "rods" are used almost exclusively in surveying, and furlongs are used almost exclusively in stories about medieval times. At the same time, although the initial discussion is about converting between Imperial and metric measures, the only metric unit you seem to support is the centimeter, not such common measures as meters or kilometers.

I see a few points that seem open to improvement.

First of all, in some cases you document the "contract" based on the type of the input and output ("number -> number") but in others the unit of measure ("Miles -> furlongs").

The units of measure are already documented directly in the function's name, so the type is the only one that really makes much sense to document in comments. For that, you'd probably be better off with a single header documenting the contract for all the functions.

Second, essentially all of these are really just multiplying/dividing the input by a factor, and returning the result. As such, it would probably make more sense to pass the units to convert as parameters. Then you'd have a single function that looked up the correct factor from a table and returned the result based on that.

Third, I'd rather see a function that actually compared results and told me whether the test passed or failed. As it stands now, I'm left to scan through the results myself to see whether the functions worked correctly or not.

Finally, the units you've chosen to support seem rather strange, at least to me. "rods" are used almost exclusively in surveying, and furlongs are used almost exclusively in stories about medieval times. At the same time, although the initial discussion is about converting between Imperial and metric measures, the only metric unit you seem to support is the centimeter, not such common measures as meters or kilometers.

P.S. The basic premise in the exercise is also somewhat incorrect: not all the rest of the world uses the metric system. For example, Imperial measures are still used quite a bit in the UK, and in some other former UK colonies as well.

Source Link
Jerry Coffin
  • 33.6k
  • 4
  • 75
  • 143

I see a few points that seem open to improvement.

First of all, in some cases you document the "contract" based on the type of the input and output ("number -> number") but in others the unit of measure ("Miles -> furlongs").

The units of measure are already documented directly in the function's name, so the type is the only one that really makes much sense to document in comments. For that, you'd probably be better off with a single header documenting the contract for all the functions.

Second, essentially all of these are really just multiplying/dividing the input by a factor, and returning the result. As such, it would probably make more sense to pass the units to convert as parameters. Then you'd have a single function that looked up the correct factor from a table and returned the result based on that.

Third, I'd rather see a function that actually compared results and told me whether the test passed or failed. As it stands now, I'm left to scan through the results myself to see whether the functions worked correctly or not.

Finally, the units you've chosen to support seem rather strange, at least to me. "rods" are used almost exclusively in surveying, and furlongs are used almost exclusively in stories about medieval times. At the same time, although the initial discussion is about converting between Imperial and metric measures, the only metric unit you seem to support is the centimeter, not such common measures as meters or kilometers.