Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name Refactoring #29

Closed
3 of 5 tasks
Spartan322 opened this issue Apr 13, 2020 · 7 comments
Closed
3 of 5 tasks

Name Refactoring #29

Spartan322 opened this issue Apr 13, 2020 · 7 comments

Comments

@Spartan322
Copy link
Collaborator

Spartan322 commented Apr 13, 2020

This issue pertains to names we have yet to refactor and are or should consider for refactoring. Please post refactoring suggestions below.

This list will stay open perpetually as long as refactoring for naming is considered.

Currently:

  • SaveLoaderReplacement, ContentLoaderReplacement, ReplacementCommons into new Replacement namespace (Add ActionsLoader Replacement #27)
  • SaveLoaderReplacement:LoadFaction -> ReplacementCommons:LoadFaction
  • ContentLoaderReplacement -> ComputerLoaderReplacement
  • Game.OS.Extensions:Write -> WriteLine (12ea319)
  • Game.OS.Extensions:WriteSingle -> WriteAppend (12ea319)
@Spartan322 Spartan322 self-assigned this Apr 13, 2020
@Spartan322
Copy link
Collaborator Author

Spartan322 commented Apr 13, 2020

Put SaveLoaderReplacement and ContentLoaderReplacement into a Replacement namespace and remove Replacement from their name.

Add more methods to ReplacementCommons, and put it in the Replacement namespace for dual applicable functionality which still needs to include:

  • SaveLoaderReplacement:LoadFaction

@Spartan322 Spartan322 pinned this issue Apr 13, 2020
@Fayti1703
Copy link
Contributor

Fayti1703 commented Apr 13, 2020

  • ContentLoader(Replacement) to ComputerLoader(replacement)

Debatably:

  • SaveLoader:LoadComputer -> ComputerLoader:LoadFromSave?

@Spartan322
Copy link
Collaborator Author

Spartan322 commented Apr 13, 2020

@Fayti1703 I think it probably be better to keep the save loading in a separate class since we have to treat it differently already and it would really bunch up the ComputerLoader class, and I think it be easier to understand there are distinctions of hooking into save loading and computer loading. (even though we do need to keep track of content and saves when loading them) Unless we separate the replacement loading methods from the hook in functionality, perhaps we could also create classes for the Internal SaveHooks and ComputerHooks that take the place of directly loading anything in the load methods.

@Spartan322
Copy link
Collaborator Author

  • Game.OS.Extensions.Write -> WriteLine
  • Game.OS.Extensions.WriteSingle -> WriteAppend

Spartan322 added a commit that referenced this issue Apr 19, 2020
Inlined InstanceOverrideDisplay to Executables.Instance
Renamed Game.OS.Extensions' WriteSingle -> WriteAppend
Renamed Game.OS.Extensions' Write -> WriteLine
Addresses #29
@Spartan322
Copy link
Collaborator Author

Spartan322 commented Sep 23, 2020

Suggestions:

  • EventExecutor -> XmlFragmentParser
  • ParsedTreeExecutor -> XmlElementParser
  • IExecutor -> XmlSegementParser (turn into a class that XmlFragmentParser and XmlElementParser will both derive from)
  • EventReader -> ? (any suggestions on new name or removal/replacement)

@Arkhist
Copy link
Owner

Arkhist commented Sep 23, 2020

I totally agree with the suggestions.
As for "EventReader", I don't know :/

@Spartan322
Copy link
Collaborator Author

Perhaps the best option would be to remove EventReader, add everything in it to XmlFragmentParser instead, we have no reason to separate them anyway and it doesn't change how anything is built otherwise.

@Arkhist Arkhist closed this as completed Jul 14, 2021
@Arkhist Arkhist unpinned this issue Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants