opengrok icon indicating copy to clipboard operation
opengrok copied to clipboard

use hashmap for groups too

Open tulinkry opened this issue 8 years ago • 30 comments

projects are using hashmap (#1543) for fast lookup, the same should be implemented for groups

tulinkry avatar May 10 '17 10:05 tulinkry

I would like to work on this. Please let me know if required. Thanks!

asharma984 avatar Oct 09 '20 05:10 asharma984

Feel free to take it !

vladak avatar Oct 09 '20 08:10 vladak

It's a free food!

tulinkry avatar Oct 09 '20 09:10 tulinkry

I would like to work on this.

herkura avatar Feb 27 '21 03:02 herkura

@vladak @tulinkry I am new to this so can you guide me on where should I start? I know basic java.

herkura avatar Feb 27 '21 03:02 herkura

The goal is to convert groups in Configuration to use HashMap. See https://github.com/oracle/opengrok/blob/5f88acb4406c8e728ddf626ec6d37eb8b5d81e34/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java#L151-L152

vladak avatar Feb 27 '21 09:02 vladak

Thanks, @vladak, I would like to contribute too.

herkura avatar Feb 27 '21 11:02 herkura

so basically I have to convert the Set into a hash map? @vladak

herkura avatar Feb 28 '21 06:02 herkura

so basically I have to convert the Set into a hash map? @vladak

Yep !

vladak avatar Feb 28 '21 19:02 vladak

@vladak I should use Map<String, String> or Map<String, Group>? Please help

herkura avatar Mar 01 '21 07:03 herkura

I am actually referring to this source: https://stackoverflow.com/questions/16108734/convert-setmap-entryk-v-to-hashmapk-v#:~:text=6%20Answers&text=There%20is%20no%20inbuilt%20API,using%20Entry%20fill%20in%20map.&text=Returns%20a%20Set%20view%20of,set%2C%20and%20vice%2Dversa. Hope I am doing it right? @vladak

herkura avatar Mar 01 '21 07:03 herkura

@vladak I should use Map<String, String> or Map<String, Group>? Please help

the latter. It should mimic what is done for projects.

vladak avatar Mar 01 '21 08:03 vladak

@vladak sir, I am sharing the code snippet can you please look into it and guide me if I have done it wrong? //Converting set into map private Map<String ,Group> group_map = new HashMap<>(); // filling the map private Set<Entry<String,Group>> groups = group_map.entrySet(); private Map<String,Group> new_group = new HashMap<>(); // iterating through the set for(Entry<String,Group> entry : groups) { mapFromSet.put(entry.getKey(),entry.getValue()); }

herkura avatar Mar 01 '21 09:03 herkura

No, you need to convert the code so that it uses the HashMap natively, not just a conversion from the existing set.

vladak avatar Mar 01 '21 09:03 vladak

so @vladak sir you mean that I have to convert only these two lines? private Map<String, Project> projects; // project name -> Project private Set<Group> groups; and it uses Hash map natively? Sorry sir I know I have been asking a lot of questions since it's my first time contributing to a proper code kindly acknowledge.

herkura avatar Mar 01 '21 09:03 herkura

Yep, begin with:

private Map<String, Project> projects; // project name -> Project 
private Map<String, Group> groups; // group name -> Group

and adapt the rest of the code.

vladak avatar Mar 01 '21 09:03 vladak

oh ok, I thought this only initially but then referred to the net. Thanks a lot sir@vladak

herkura avatar Mar 01 '21 09:03 herkura

@vladak sir can you please now review my Pull Request?

herkura avatar Mar 01 '21 09:03 herkura

@vladak sir is it fine sir or any changes are required ?

herkura avatar Mar 01 '21 10:03 herkura

You need to see how exactly is the group member used and adapt the rest of the code. This is definitely not a one liner change. Read the code extensively first and become familiar with how the project groups work. Then proceed to make any changes.

Also, run the tests before submitting PRs.

vladak avatar Mar 01 '21 10:03 vladak

ok sir

herkura avatar Mar 01 '21 10:03 herkura

Hello @vladak @tulinkry , this is my first time contributing to an Open Source project, and am still learning the process. I have tried to make the desired changes as told in the issue and was wondering how I can test if my changes are working. I would really appreciate your mentoring and any help that you can provide. Thanks!

Edit: I have never worked on a production level project. Hoping to make a useful contribution while learning new things and improving myself.

adityamathur97 avatar Mar 11 '21 03:03 adityamathur97

Seems like we will have races of who will fix this first :-) Feel free to submit a PR. As for the testing, you can totally do it yourself. Just create a setup with project groups, see https://github.com/oracle/opengrok/pull/3466

vladak avatar Mar 11 '21 09:03 vladak

Yep, begin with:

private Map<String, Project> projects; // project name -> Project 
private Map<String, Group> groups; // group name -> Group

and adapt the rest of the code.

do I just need to make changes in Configuration.java so that the above changes work or should I make changes to methods like setGroup(Set<Group> groups) -> setGroup(Map<String, Group> groups). I am confused as to what extent I should make changes!

adityamathur97 avatar Mar 11 '21 12:03 adityamathur97

Definitely the setters/getters need to adapt. And any other code that uses the value returned from the getter. Just keep bubbling/propagating this.

vladak avatar Mar 11 '21 13:03 vladak

Hi @adityamathur97 are you still working on this ? or can I take it ?

ghost avatar Nov 12 '21 08:11 ghost

As issue is still open I will pick this up.

MrRa1n avatar Dec 28 '21 13:12 MrRa1n

Is this issue closed?

IordanisKokk avatar Feb 06 '22 11:02 IordanisKokk

The status says open and there is PR #3873 in progress. Looks like there is a bit of contention for this one :-) There are lots of other, potentially more interesting issues to work on.

vladak avatar Feb 07 '22 09:02 vladak

Hi, Is it still open? Can I contribute to it? I have gone through the configuration.java file and found that changes will be required on multiple place on the file. Can I start working on it?

Rajdeep-Paul-117 avatar Aug 14 '22 16:08 Rajdeep-Paul-117