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

Consistent naming #349

Closed
DraTeots opened this issue Aug 28, 2024 · 5 comments
Closed

Consistent naming #349

DraTeots opened this issue Aug 28, 2024 · 5 comments

Comments

@DraTeots
Copy link
Collaborator

DraTeots commented Aug 28, 2024

Proposal: make JANA2 API naming consistent.

I believe, as of now, the most inconsistent part is member functions naming. All major API classes have PascalCase members, but some groups of classes have every function in snake_case (e.g. Topology or Engine folders).

Whatever JANA2 is going to end with: PascalCase or snake_case doesn't matter compared to how it looks now:

    auto app = new JApplication(new JParameterManager()); 
    app->AddPlugin("my_plugin");
    app->ProvideService(s);

   // And then...
    auto processor_arrow = new JEventProcessorArrow("processors", event_queue, nullptr, topology->event_pool);
    processor_arrow->add_processor(processor);
    block_source_arrow->attach(block_disentangler_arrow);
    block_disentangler_arrow->attach(processor_arrow);
    for (auto proc : topology->component_manager->get_evt_procs()) {
        processor_arrow->add_processor(proc);
   }
   
   // But back again
   app->Initialize();
   app->GetComponentSummary()
   app->GetService<RootFile_service>()
   
   // And this is the same framework...
@nathanwbrei
Copy link
Collaborator

Not inconsistent actually! The PascalCase methods are public-facing, and the snake_case methods are not. The example you pasted uses non-public-facing methods because the public ones didn't exist at the time.

@DraTeots
Copy link
Collaborator Author

So how to rewrite a piece above with public API and consistent naming?

This particular code:

https://github.com/JeffersonLab/JANA4ML4FPGA/blob/main/src/executables/jana4ml4fpga/CDAQTopology.h

@DraTeots
Copy link
Collaborator Author

DraTeots commented Nov 1, 2024

What about this one?

void acquire_services(JServiceLocator *) override;

Is it public-facing enough to be in every user written service?

@DraTeots DraTeots reopened this Nov 1, 2024
@DraTeots
Copy link
Collaborator Author

DraTeots commented Nov 2, 2024

Ok =) Found this in the source:

    // This will be deprecated eventually
    virtual void acquire_services(JServiceLocator*) {};

Thanks!

@DraTeots DraTeots closed this as completed Nov 2, 2024
@nathanwbrei
Copy link
Collaborator

acquire_services() will be replaced by Init(), once I update eicrecon and hd_recon so as not to produce tons of deprecation warnings. However, Init() itself is less necessary because parameters and services should be injected using the Parameter<>() and Service<> helpers, just like JOmniFactory. As far as I'm aware, all components now support injecting parameters and services this way. This is part of #276.

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

No branches or pull requests

2 participants