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

In AbstractHttpTransport _sessions should be protected instead of private #793

Open
barbossusus opened this Issue Sep 25, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@barbossusus

barbossusus commented Sep 25, 2018

Hi,

Regarding cometd 3.1.4:
When we extend AbstractHttpTransport we would like to have access to the _sessions class variable from the subclass. For instance, the _logger class variable is already allowing access to subclasses.

public abstract class AbstractHttpTransport extends AbstractServerTransport {
...
protected final Logger _logger = LoggerFactory.getLogger(getClass());
private final ThreadLocal currentRequest = new ThreadLocal<>();
protected final Map<String, Collection> _sessions = new HashMap<>();

private final ConcurrentMap<String, AtomicInteger> _browserMap = new ConcurrentHashMap<>();
...

@sbordet

This comment has been minimized.

Show comment
Hide comment
@sbordet

sbordet Sep 25, 2018

Member

Why? I'm not keen to make _sessions a public field.

Member

sbordet commented Sep 25, 2018

Why? I'm not keen to make _sessions a public field.

@barbossusus

This comment has been minimized.

Show comment
Hide comment
@barbossusus

barbossusus Sep 25, 2018

Hi Simone,
It would help to have it protected, not public.

barbossusus commented Sep 25, 2018

Hi Simone,
It would help to have it protected, not public.

@sbordet

This comment has been minimized.

Show comment
Hide comment
@sbordet

sbordet Sep 25, 2018

Member

You have not explained why yet.

Member

sbordet commented Sep 25, 2018

You have not explained why yet.

@barbossusus

This comment has been minimized.

Show comment
Hide comment
@barbossusus

barbossusus Sep 25, 2018

For instance, if I want to override "findCurrentSessions" there is no way I can reference _sessions and use it part of my logic.

barbossusus commented Sep 25, 2018

For instance, if I want to override "findCurrentSessions" there is no way I can reference _sessions and use it part of my logic.

@sbordet

This comment has been minimized.

Show comment
Hide comment
@sbordet

sbordet Sep 25, 2018

Member

You have explained what, not why.

Just because you continue to say "I want _sessions protected, I want to override findCurrentSessions(), I want ... I want ...` you are not going to convince me.

Produce a detailed use case, what you are actually trying to do, why the current situation is not enough for what you want to do, present the alternative you have to code if _sessions will remain private, and why you think making _sessions protected will help you.

Member

sbordet commented Sep 25, 2018

You have explained what, not why.

Just because you continue to say "I want _sessions protected, I want to override findCurrentSessions(), I want ... I want ...` you are not going to convince me.

Produce a detailed use case, what you are actually trying to do, why the current situation is not enough for what you want to do, present the alternative you have to code if _sessions will remain private, and why you think making _sessions protected will help you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment