Skip to content

#8590 Refactor: Remove all methods from SmilesFormatter class except one - isContainsCoordinates - #8610

Merged
AlexeyGirin merged 3 commits into
masterfrom
copilot/refactor-smilesformatter-class
Nov 20, 2025
Merged

AlexeyGirin merged 3 commits into
masterfrom
copilot/refactor-smilesformatter-class

Conversation

Copilot AI commented Nov 20, 2025 •

Copy link
Copy Markdown
Contributor

How the feature works? / How did you fix the issue?

Removed all instance methods and dependencies from SmilesFormatter class, retaining only the static utility method isContainsCoordinates() which is the sole usage point in serverFormatter.ts.

Additionally, removed all unused SMILES serialization code that was only used by the deleted SmilesFormatter methods, including the entire SmiSerializer class and related helper modules.

Changes Made:

Commit 1: Refactored SmilesFormatter class (45 lines removed)

  • Removed instance methods: getStructureFromStructAsync, getStructureFromStringAsync
  • Removed constructor and all private fields
  • Removed implements StructFormatter interface declaration
  • Removed all import statements (no longer needed)

Commit 2: Removed unused SMILES serialization code (2,174 lines removed)

  • Removed SmiSerializer class
  • Removed Smiles class with all prototype methods: writeAtom(), updateSideBonds(), saveMolecule(), writeCycleNumber(), markCisTrans(), calcBondDirection(), writeRadicals(), isBondInRing()
  • Removed helper modules: cis_trans.js (219 lines), dfs.js (187 lines), stereocenters.js (729 lines)
  • Removed test files: dfs.test.js, smiles.test.js
  • Removed type definitions and exports
  • Deleted entire smi directory from domain/serializers

Before (64 lines):

export class SmilesFormatter implements StructFormatter {
  readonly #smiSerializer: SmiSerializer;
  // ... more fields
  
  constructor(...) { /* ... */ }
  async getStructureFromStructAsync(struct: Struct): Promise<string> { /* ... */ }
  getStructureFromStringAsync(stringifiedStruct: string): Promise<Struct> { /* ... */ }
  static isContainsCoordinates(smiles: string) { /* ... */ }
}

After (21 lines):

export class SmilesFormatter {
  static isContainsCoordinates(smiles: string) {
    return /\([\d.,;-]+\)/.test(smiles);
  }
}

Impact:

  • Total: 2,219 lines of code removed
  • Reduced circular dependencies from 264 to 260
  • All 271 unit tests pass
  • Build successful with no breaking changes

Check list

Original prompt

This section details on the original issue you should resolve

<issue_title>Refactor: Remove all methods from SmilesFormatter class except one - isContainsCoordinates</issue_title>
<issue_description>Please remove all methods from SmilesFormatter class except one - isContainsCoordinates since they are not not used any more.

Class location: packages\ketcher-core\src\application\formatters\serverFormatter.ts
</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…ates

Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor SmilesFormatter class to retain only isContainsCoordinates method Refactor: Remove unused methods from SmilesFormatter class Nov 20, 2025
Copilot AI requested a review from AlexeyGirin November 20, 2025 16:48
@AlexeyGirin

Copy link
Copy Markdown
Collaborator

@copilot remove also all functions that were used in removed methods of SmilesFormatter class if they are not user anywhere.
For example: Smiles.prototype.writeAtom = function, Smiles.prototype.updateSideBonds and etc

Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>

Copilot AI commented Nov 20, 2025

Copy link
Copy Markdown
Contributor Author

@copilot remove also all functions that were used in removed methods of SmilesFormatter class if they are not user anywhere.
For example: Smiles.prototype.writeAtom = function, `Smiles.prototype....

Removed all unused SMILES serialization code (2,174 lines) in commit 72df6bd:

  • Deleted SmiSerializer class
  • Deleted Smiles class with all prototype methods: writeAtom(), updateSideBonds(), saveMolecule(), writeCycleNumber(), markCisTrans(), calcBondDirection(), writeRadicals(), isBondInRing()
  • Deleted helper modules: cis_trans.js, dfs.js, stereocenters.js
  • Deleted entire smi directory with tests and type definitions

Build and tests pass successfully.

Copilot AI changed the title Refactor: Remove unused methods from SmilesFormatter class Refactor: Remove unused SmilesFormatter methods and SMILES serialization code Nov 20, 2025
@AlexeyGirin
AlexeyGirin marked this pull request as ready for review November 20, 2025 18:30
@AlexeyGirin AlexeyGirin changed the title Refactor: Remove unused SmilesFormatter methods and SMILES serialization code #8590 Refactor: Remove all methods from SmilesFormatter class except one - isContainsCoordinates Nov 20, 2025
@AlexeyGirin
AlexeyGirin merged commit fafeb2d into master Nov 20, 2025
17 of 21 checks passed
@AlexeyGirin
AlexeyGirin deleted the copilot/refactor-smilesformatter-class branch November 20, 2025 20:38
AlexeyGirin added a commit that referenced this pull request Nov 20, 2025
…one - isContainsCoordinates (#8610)

* Initial plan

* Refactor SmilesFormatter: remove all methods except isContainsCoordinates

Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>

* Remove unused SmiSerializer and Smiles serialization code

Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: AlexeyGirin <26869421+AlexeyGirin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: Remove all methods from SmilesFormatter class except one - isContainsCoordinates

3 participants