Is this bad coding practice? I feel like there is repetition and redundancy

by

Summary

The provided C++ temperature converter code exhibits repetition and redundancy in its function definitions and conversion logic. This leads to code bloat, reduced maintainability, and increased potential for errors.

Root Cause

  • Duplicate Conversion Logic: Each conversion (e.g., Celsius to Fahrenheit, Fahrenheit to Kelvin) is implemented as a separate function, even though the underlying calculations are often similar or share common steps.

  • Hardcoded Unit Handling: The switch statement directly handles unit conversions based on user input, leading to repetitive code blocks for each unit type.

Why This Happens in Real Systems

  • Early Learning Stage: Beginners often focus on getting functionality working, prioritizing immediate results over code structure and efficiency.
  • Lack of Abstraction: Newer programmers might not yet grasp concepts like functions as reusable building blocks or data-driven design.

Real-World Impact

  • Increased Code Size: More lines of code mean more to read, understand, and maintain.
  • Higher Maintenance Costs: Changes to conversion formulas require modifications in multiple places, increasing the risk of introducing bugs.
  • Reduced Readability: Repetitive code is harder to follow and understand, making collaboration and debugging more difficult.

Example or Code

#include 
using namespace std;

// Enum for temperature units
enum class Unit { Celsius, Fahrenheit, Kelvin };

// Function to convert between units
double convert(double value, Unit from, Unit to) {
    if (from == to) return value;

    if (from == Unit::Celsius) {
        if (to == Unit::Fahrenheit) return (value * 1.8) + 32;
        if (to == Unit::Kelvin) return value + 273.15;
    } else if (from == Unit::Fahrenheit) {
        if (to == Unit::Celsius) return (value - 32) * 5 / 9;
        if (to == Unit::Kelvin) return ((value - 32) * 5 / 9) + 273.15;
    } else if (from == Unit::Kelvin) {
        if (to == Unit::Celsius) return value - 273.15;
        if (to == Unit::Fahrenheit) return ((value - 273.15) * 1.8) + 32;
    }

    return 0.0; // Handle invalid conversions
}

int main() {
    double value;
    char unitChoice;

    cout << "Temperature Converter" << endl;
    cout <> value;

    cout << "Choose Your Unit (a, b, or c): " << endl;
    cout << "a(Celsius)" << endl;
    cout << "b(Fahrenheit)" << endl;
    cout << "c(Kelvin)" <> unitChoice;

    Unit fromUnit;
    switch (unitChoice) {
        case 'a': fromUnit = Unit::Celsius; break;
        case 'b': fromUnit = Unit::Fahrenheit; break;
        case 'c': fromUnit = Unit::Kelvin; break;
        default:  cout << "Invalid unit choice." << endl; return 1;
    }

    cout << "Converted Temperatures:" << endl;
    cout << "Celsius: " << convert(value, fromUnit, Unit::Celsius) << endl;
    cout << "Fahrenheit: " << convert(value, fromUnit, Unit::Fahrenheit) << endl;
    cout << "Kelvin: " << convert(value, fromUnit, Unit::Kelvin) << endl;

    return 0;
}

How Senior Engineers Fix It

  • Centralized Conversion Logic: Implement a single convert function that takes the input value, source unit, and target unit as parameters. This eliminates redundant code and makes updates easier.
  • Enums for Units: Use an enum to represent temperature units, improving code readability and type safety.
  • Data-Driven Approach: Store conversion formulas in a data structure (e.g., a map) for even greater flexibility and extensibility.

Why Juniors Miss It

  • Focus on Functionality: Beginners prioritize making the code work, often overlooking opportunities for optimization and code structure.
  • Limited Exposure: They might not have encountered best practices for code organization and reusability yet.
  • Fear of Complexity: Simplifying code through abstraction can seem daunting at first.

Key Takeaway: Strive for code reusability, modularity, and readability to create maintainable and efficient programs.

Leave a Comment