Skip to content

#3014 multiple dose application naming#3379

Merged
rwmcintosh merged 2 commits intodevelopfrom
3014-multiple-dose-application-naming
Jan 22, 2026
Merged

#3014 multiple dose application naming#3379
rwmcintosh merged 2 commits intodevelopfrom
3014-multiple-dose-application-naming

Conversation

@benjaperez1983
Copy link
Contributor

Fixes #3014

Description

When creating multiple dose application and exporting it to MoBi, the index in the naming does not have the same amount of numbers, leading to confusing order.

Type of change

Please mark relevant options with an x in the brackets.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires documentation changes (link at least one user or developer documentation issue):
  • Algorithm update - updates algorithm documentation/questions/answers etc.
  • Other (please describe):

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Integration tests
  • Unit tests
  • Manual tests
  • No tests required

Reviewer checklist

Mark everything that needs to be checked before merging the PR.

  • Check if the code is well documented
  • Check if the behavior is what is expected
  • Check if the code is well tested
  • Check if the code is readable and well formatted
  • Additional checks (document below if any)
  • Check if documentation update issue(s) are created if the option This change requires a documentation update above is selected

Screenshots (if appropriate):

image

Questions (if appropriate):

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the naming inconsistency in multiple dose applications when exporting to MoBi. Previously, application indices lacked uniform padding (e.g., "Application_1", "Application_10"), causing confusing sorting. The fix ensures indices are zero-padded to maintain consistent width (e.g., "Application_001", "Application_010").

Changes:

  • Modified addProtocol method to calculate and apply uniform zero-padding to application indices based on total count
  • Converted schema items enumeration to a list to enable count-based width calculation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@greptile-apps
Copy link

greptile-apps bot commented Jan 22, 2026

Greptile Overview

Greptile Summary

Fixed multiple dose application naming to use zero-padded indices, ensuring proper lexicographic ordering when exported to MoBi.

  • Changed from simple incrementing numbers (Application_1, Application_2) to zero-padded format (Application_001, Application_002, Application_100)
  • Calculates padding width dynamically based on total number of schema items
  • Ensures consistent sorting regardless of the number of applications (10, 100, 1000+)

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change is a simple, well-contained bug fix that improves naming consistency. The logic is straightforward: it converts the enumerable to a list (minor performance consideration but necessary for count), calculates the required padding width, and formats numbers with zero-padding. The implementation correctly uses the D format specifier for decimal numbers with leading zeros.
  • No files require special attention

Important Files Changed

Filename Overview
src/PKSim.Core/Services/EventBuildingBlockCreator.cs Fixed multiple dose application naming to use zero-padded indices for consistent ordering (e.g., Application_001, Application_002 instead of Application_1, Application_2)

Sequence Diagram

sequenceDiagram
    participant Client
    participant EventBuildingBlockCreator
    participant SchemaItemsMapper
    participant ApplicationFactory
    
    Client->>EventBuildingBlockCreator: addProtocol(compoundProperties)
    EventBuildingBlockCreator->>SchemaItemsMapper: MapFrom(protocol)
    SchemaItemsMapper-->>EventBuildingBlockCreator: IEnumerable<ISchemaItem>
    
    Note over EventBuildingBlockCreator: Convert to List and calculate width<br/>width = schemaItems.Count.ToString().Length
    
    loop For each schemaItem (with index)
        Note over EventBuildingBlockCreator: Format number with padding<br/>number = (index+1).ToString($"D{width}")<br/>e.g., "001", "002", ..., "100"
        EventBuildingBlockCreator->>EventBuildingBlockCreator: Create applicationName<br/>"Application_{number}"
        EventBuildingBlockCreator->>ApplicationFactory: addApplication(eventGroup, schemaItem, applicationName)
        ApplicationFactory-->>EventBuildingBlockCreator: Application created
    end
    
    EventBuildingBlockCreator-->>Client: Protocol added with zero-padded names
Loading

@rwmcintosh rwmcintosh merged commit e72e7d8 into develop Jan 22, 2026
6 checks passed
@rwmcintosh rwmcintosh deleted the 3014-multiple-dose-application-naming branch January 22, 2026 14:42
benjaperez1983 added a commit that referenced this pull request Mar 9, 2026
…multiple-dose-application-naming"

This reverts commit e72e7d8, reversing
changes made to f8c1773.
@benjaperez1983
Copy link
Contributor Author

@PavelBal V13 Branch already has this changes since there has been a sync between develop and V13.
But I`ve created this PR to remove the changes from Develop branch.

msevestre pushed a commit that referenced this pull request Mar 9, 2026
…multiple-dose-application-naming" (#3442)

This reverts commit e72e7d8, reversing
changes made to f8c1773.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple dose application naming

4 participants