#43809: Introduce struct for requirement targets Open Date: 2022-02-08 00:24 Last Update: 2022-02-08 00:44 URL for this Ticket: https://osdn.net//projects/freeciv/ticket/43809 RSS feed for this Ticket: https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=43809 --------------------------------------------------------------------- Last Changes/Comment on this Ticket: 2022-02-08 00:44 Updated by: alienvalkyrie * Owner Update from (None) to alienvalkyrie Comment: Question for the more experienced folks (wrt Freeciv code in particular, and C in general), since it doesn't seem to come up elsewhere in the codebase: To replace a call like struct tile *ptile = NULL; if (pcity != NULL) { ptile = city_tile(pcity); } if (is_req_active(pplayer, NULL, pcity, pimprove, ptile, NULL, NULL, NULL, NULL, NULL, preq, RPT_CERTAIN)) { /* ... */ }would a better replacement be struct requirement_targets tgts; req_targets_clear(&tgts); tgts.player = pplayer; tgts.city = pcity; tgts.improvement = pimprove; if (pcity != NULL) { tgts.tile = city_tile(pcity); } if (is_req_active(&tgts, NULL, preq, RPT_CERTAIN)) { /* ... */ }or a C99 compound literal, a la struct requirement_targets tgts; struct tile *ptile = NULL; if (pcity != NULL) { ptile = city_tile(pcity); } tgts = (struct requirement_targets) { .player = pplayer, .city = pcity, .improvement = pimprove, .tile = ptile }; if (is_req_active(&tgts, NULL, preq, RPT_CERTAIN)) { /* ... */ }Compound literals do show up for union types occasionally (typically universals), but I can't recall seeing them used for structs. (The leftover NULL in the two replacements is the other_player, left separate for reasons including both actions code and future considerations.) --------------------------------------------------------------------- Ticket Status: Reporter: alienvalkyrie Owner: alienvalkyrie Type: Patches Status: Open [Owner assigned] Priority: 5 - Medium MileStone: 3.1.0 Component: General Severity: 5 - Medium Resolution: None --------------------------------------------------------------------- Ticket details: Currently, calls to is_req_active() (and accordingly, to are_reqs_active() and is_enabler_active()) are highly inscrutable, given the number of arguments (many of which are often NULL). More problematically, it's impossible to add new kinds of requirement targets without changing every call site (usually by adding another NULL at some spot in the call). Solution: Add a new struct requirement_targets which contains all of them and make aforementioned functions take a pointer to it. Individual call sites only need to set their relevant fields (by field name ~> better readability), so new fields can be added to the struct with less hassle. Possible performance considerations: Additional layer of indirection via pointer to requirement_targets struct Individual requirement targets don't need to be copied anymore when e.g. are_reqs_active() calls is_req_active() Both of these are probably relatively minor. -- Ticket information of Freeciv project Freeciv Project is hosted on OSDN Project URL: https://osdn.net/projects/freeciv/ OSDN: https://osdn.net URL for this Ticket: https://osdn.net/projects/freeciv/ticket/43809 RSS feed for this Ticket: https://osdn.net/ticket/ticket_rss.php?group_id=12505&tid=43809