tocc icon indicating copy to clipboard operation
tocc copied to clipboard

Issue67

Open hlYassine opened this issue 10 years ago • 10 comments

I'm thinking of adding others wild cards ( '?', '{ }', ...) in the future, I think I'd use the "Trie" algorithm, not sure though. Anyway, now it's only the asterisk wild card that is implemented in this commit. I've used Singleton design pattern in the submited code, so if you don't like it I'll find another way.

hlYassine avatar Oct 18 '14 14:10 hlYassine

Sorry I found a bug on this branch. don't merge it until the next commit. thanks

hlYassine avatar Oct 18 '14 17:10 hlYassine

done :)

hlYassine avatar Oct 18 '14 23:10 hlYassine

Wow! Thanks for all the work! I should take a deeper look at what you've done (:

aidin36 avatar Oct 19 '14 16:10 aidin36

The overal of you design is excellent. It was a good idea to have a WildCardManager, so we can support other wild cards in the future.

There's some little problems with the implementation.

  1. Why you create a Singletons class and add all the singleton classes into it? We could simply add a get_instance static method to WildCardManager. Or, WildCardManager could have just some static methods.

  2. Seems that business of WildCardManager and WildCard mixed up. WildCardManager shouldn't do somethings like checking if and expression contain a wild card char (wild_card_manager.cpp, line 39), neigther it should parse that path to split directory and file name. In this way, I can never write a WildCard that be able to process something like /path/2014-*/DSC.JPG.

WildCardManager should be only a container of WildCards. Each WildCard should have an expand method. This method will be decide if any wild card character is in the specified path, and how it should be expanded. WildCardManager loops through the list of WildCards, and calls the expand method of them, collects the result, and returns it.

In other words: WildCardManager tells BraceWildCard: Expand this: /path/2014-{02..06}/*.JPG. It returns a list of strings like /path/2014-02/*.JPG, /path/2014-03/*.JPG, etc. Then, WildCardManager tells AsteriskWildCard: Expand this: /path/2014-02/*.JPG, it returns a list of JPGs in that directory. Then again, WildCardManager tells AsteriskWildCard to expand the second path: /path/2014-03/*.JPG. And it goes like that for each path. If a WildCard found out that there's no wild card char in the path, it do nothing.

Encapsulation ;-)

If I couldn't explain it clearly, I can send you a UML and pseudo code.

aidin36 avatar Oct 20 '14 03:10 aidin36

Also, please consider formatting. Things like space after if, and no space after parenthesis, etc. Seems that you forget our coding style when you were away ;-)

aidin36 avatar Oct 20 '14 03:10 aidin36

Thanks my friend for the remarks :). you are right. In fact, I'm working on a design to implement all wildcards, I've seen it'll take some time this is why I submited this which lists only files. I'll fix the formatings, sorry I mix the working code style with tocc style :D .

hlYassine avatar Oct 26 '14 14:10 hlYassine

:D Take your time, my firend (:

aidin36 avatar Oct 27 '14 02:10 aidin36

What's up, Yassine? (:

aidin36 avatar Nov 18 '14 04:11 aidin36

hello Aidin, hope you're fine. actually I'm still workin on the issue, I'll be able to finish it soon, then I'll refactor the code and submit it.

hlYassine avatar Nov 18 '14 12:11 hlYassine

Thanks Yassine (: I'm waiting for that.

aidin36 avatar Nov 19 '14 16:11 aidin36