3

I am making an OO design for an app. So far I've come with this, I looks like it could work, but I am not sure of correctness of its design.

Old design enter image description here

EDIT

Okay so, I used factory pattern, and it looks good to me. Is this good design ? enter image description here

You are building application that will load data from several different advertising systems and then store the data into database, so it will be possible to analyze them.

You need to take in mind that every advertising system has different structure of report: Different named columns, different order of columns, different date formats. Also data from ad systems are in different data formats(JSON,CSV,XML),

Reports from each system contains different amount of columns, our application is interested only in some of them: date, ad_campaing, ad_group, keyword, impressions, price (in every system they have different name)

5
  • Could you find a design pattern that is close? You may get more responses if you are able to have a more specific question about a particular pattern. Commented Dec 1, 2016 at 11:14
  • I am going through them right now and trying to think of which would be good. But I think to find one that will fit, will require to change the whole logic of what I have. Commented Dec 1, 2016 at 11:20
  • So actually you are saying that Bing and Yahoo are AdSystems? Commented Dec 1, 2016 at 11:51
  • Right... The Bing/Adwords/Yahoo classes technically don't need to be there, I only have them there because in reportColumns I saved which reportColumns I am interested in. I could saved it in config file for example. Commented Dec 1, 2016 at 11:58
  • 'looks like it could work', for us to know for sure we would need some requirements. Commented Aug 8, 2017 at 19:20

3 Answers 3

1

Your report data layer is not Open-Closed (https://en.m.wikipedia.org/wiki/Open/closed_principle), it knows all formats and must be changed in order to be extended with new formats.

The design didn't explain how the reports are constructed, which is the heart of this system. This is something you may wish to address at this stage.

1
0

I would go with

Advert { string id; var ad_group; var keyword; var impressions; var price ; } AdRepository_MyDatabase { InsertAdvert(Advert advert); GetAdverts(); } //this class is the only thing that knows about the bing format and how to //get the bing adverts and convert them to our adverts AdRepository_Bing { List<Advert> GetAdverts(); } AdRepository_Google { List<Advert> GetAdverts(); } etc WorkerService { DownloadAdverts() { foreach(repo in RepositoryCollection) { var ads = repo.GetAdverts(); foreach(ad in ads) { mydatabaseRepo.InsertAdvert(ad); } } } } 
2
  • Yea, except some class for loading the data(downloading/opening files) it's basically same as what I did. And I am missing some class that will glue it together Commented Dec 2, 2016 at 12:04
  • So, you are asking on how to connect the ReportLoader with an AdvertisingFactory? Commented Dec 10, 2016 at 8:38
0

I do believe it is a good architecture, but I came with a question, how would you say to the AdvertisingFactory how to convert the string to a report, I hope I understood the question right. So it came to me an implementation of an Strategy Pattern:

Strategy Pattern

There the one that does your conversion is the ReportMapper, because I didn't found another name, and the factory wraps it all.

3
  • I get report data in form of array, and in report class I select only values that I need with selectData method Commented Dec 11, 2016 at 2:26
  • That I didn't specify because you had already done so in the question. Commented Dec 11, 2016 at 2:34
  • The Mapper exists just to convert the string to report, there you report loader converts it to array. Commented Dec 11, 2016 at 2:37

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.