Refactor: config_create() 11-parameter constructor is unmaintainable #150

New Issue

2026-07-29T18:21:33+02:00

TapTap commented

2026-07-29 18:21:33 +02:00

Description

The config_create() function in src/shared/config.c (line 12) takes 11 positional parameters — a mix of char*, bool, and int. At the call site in client_cli.c:77-78, the argument list spans two lines and is nearly impossible to read correctly without counting parameters:

Config* config = config_create(str_dup(PROTOCOL_VERSION), NULL, NULL, save_to_disk, false, false,
                               false, false, 5, false, 0);

This is fragile: adding a new field reorders or shifts all subsequent arguments, and nothing prevents the wrong false/NULL from being passed in the wrong slot. Every time a new field is added to the Config struct, this constructor signature must change.

A better approach would be:

Use a zero-initialized struct literal + setters, or
Adopt a builder pattern, or
Use designated initializers with a Config initializer macro.

Location

src/shared/config.c:12-57 and src/client/client_cli.c:77-78

Suggested Fix

Replace the 11-parameter constructor with:

A Config* config_create_default(void) that returns a config with all defaults.
Individual setter functions or direct field assignment with documented defaults.
Or a macro: #define CONFIG_INIT { .version = str_dup(PROTOCOL_VERSION), .compression_level = 5, ... }

This eliminates the risk of parameter misordering and makes call sites self-documenting.

Severity

Medium

Category

Quality / Maintainability

## Description The `config_create()` function in `src/shared/config.c` (line 12) takes **11 positional parameters** — a mix of `char*`, `bool`, and `int`. At the call site in `client_cli.c:77-78`, the argument list spans two lines and is nearly impossible to read correctly without counting parameters: ```c Config* config = config_create(str_dup(PROTOCOL_VERSION), NULL, NULL, save_to_disk, false, false, false, false, 5, false, 0); ``` This is fragile: adding a new field reorders or shifts all subsequent arguments, and nothing prevents the wrong `false`/`NULL` from being passed in the wrong slot. Every time a new field is added to the `Config` struct, this constructor signature must change. A better approach would be: 1. Use a zero-initialized struct literal + setters, or 2. Adopt a builder pattern, or 3. Use designated initializers with a `Config` initializer macro. ## Location `src/shared/config.c:12-57` and `src/client/client_cli.c:77-78` ## Suggested Fix Replace the 11-parameter constructor with: - A `Config* config_create_default(void)` that returns a config with all defaults. - Individual setter functions or direct field assignment with documented defaults. - Or a macro: `#define CONFIG_INIT { .version = str_dup(PROTOCOL_VERSION), .compression_level = 5, ... }` This eliminates the risk of parameter misordering and makes call sites self-documenting. ## Severity Medium ## Category Quality / Maintainability

TapTap referenced this issue from a commit

2026-07-29 18:34:12 +02:00

refactor: config_create(), main(), send_files() - reduce duplication and complexity

TapTap referenced this issue

2026-07-29 18:34:15 +02:00

refactor: config_create(), main(), send_files() — reduce duplication and complexity (#149, #150, #151) #155

TapTap referenced this issue

2026-07-29 18:36:42 +02:00

Batch fix: NULL checks, input validation, and code refactoring (#152, #149, #150, #151) #171

TapTap referenced this issue from a commit

2026-07-29 19:11:18 +02:00

refactor: resolve code quality issues (#149, #150, #151, #152)