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

New Baseline #33

Merged
merged 35 commits into from
May 20, 2019
Merged

New Baseline #33

merged 35 commits into from
May 20, 2019

Conversation

FuzzicalLogic
Copy link
Contributor

**This PR need not be deployed. There are no functional benefits to this PR. **

While RssService changed considerably, the functionality is largely the same. This PR is mostly to provide a new base for implementing a branching strategy on my fork.

Sonuva... Grrr.... Tabify screwed with commits.

1) Localize LeaderboardData to BuildLeaderboardAsync
2) Change return value of GetLeaderBoardDataAsync from Task to Task<LeaderboardData>
3) Inject data from BuildLeaderboardAsync into: BuildTopClassEmbed, BuildFirstTopAscendancyEmbed, BuildSecondTopAscendancyEmbed, BuildDiscordOnlyEmbed.
4) Fix URL scheme in GetLeaderBoardDataAsync
In a situation where I need original database to compare to new tampered with database.
Luckily no keys were exposed.
Each function should do one thing. Pass the result to the next step
Move filtering from BuildRssFeedAsync to its own function
From excludeRecentPosts
Change forEach to list.
Move from BuikdRssFeedAsync
Marks completion of Task chain
Save database after all writes have completed.
Sending to Channels done separately
Move to outside of loop (only needs once per RSS feed.
Move code from BuildRssFeedAsync
Use a `List<object>` to transfer the built messages via DI to `SendToChannel()`
Allows higher functions to respond to errors appropriately.
Catching exceptions in the loop will avoid breaking the bot and allow only the broken builders/urls to continue on, while still providing debugging information
@FuzzicalLogic
Copy link
Contributor Author

Generally, I prefer to know what subtype I'm dealing with before I know what it applies to.

So id tells me what type of number it is, which is generally way more important for functionality than knowing where it goes. The "model" or "class" handles where it goes, but the subtype determines what I can do with it (if that makes sense) and what I must always keep in mind.

It also allows me to group similar or related vars without having to read every varname... for example, idGuild and urlGuild and rssGuild are might be right next to each other. But in a guild only function, I'd only have to see the first few characters. For me, it's about reading and thought efficiency.

@Kyle-Undefined
Copy link
Owner

@FuzzicalLogic personally I'm against Hungarian style, and while I understand what you're saying, would rather not have that if all possible.

@FuzzicalLogic
Copy link
Contributor Author

@Kyle-Undefined That's fair, and I'll fix it in the next PR. I would prefer to do it with the next PR because it will coincide well with the introduction of the STYLE_GUIDE.md. I hope that is alright.

@Kyle-Undefined
Copy link
Owner

@FuzzicalLogic sounds good to me dude. I've just never been a fan of it, that's all. I'll hopefully be able to test tomorrow and merge.

@FuzzicalLogic
Copy link
Contributor Author

That's not a problem. I can work in a variety of styles. Unless it has a computational impact, I am very very flexible. If you see a style that I introduce without thought, definitely let me know. If you're interested as to why, please ask. And if you don't like it, please tell me.

As a note: I don't code in hungarian notation primarily... just for subtypes. There's a distinct difference between noting a base type and a subtype. Base Types which can be easily determined in declaration and whose semantics can change according to environment or language. Subtypes like id are agnostic computationally, and are consistent across coding environments.

@FuzzicalLogic
Copy link
Contributor Author

Also, I'm going to open an Issue regarding Styles and Conventions, simply so we can lock those down in case others decide to contribute. That way we don't have to discuss it in the PRs or the discord. Also we can take time in case one of us feels strongly about an idea. Of course it will be closed as soon as the STYLE_GUIDE is complete.

The reason is because there are many code types/patterns that I haven't seen in this example of code, so I want to have a forum for which to ask, discuss and get feedback for as they come up.

FuzzicalLogic added a commit to FuzzicalLogic/PoE-Bot that referenced this pull request May 18, 2019
@Kyle-Undefined
Copy link
Owner

@FuzzicalLogic yeah a style guide would be perfect. Personally I like method params to be camel case. But that's just me. We can flesh everything out and clean up what we come up with for the style guide as we go.

Copy link
Owner

@Kyle-Undefined Kyle-Undefined left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging from phone, haven't had chance to test locally. Looked fine from what I've seen.

@Kyle-Undefined Kyle-Undefined merged commit 53ccf01 into Kyle-Undefined:master May 20, 2019
FuzzicalLogic added a commit to FuzzicalLogic/PoE-Bot that referenced this pull request May 20, 2019
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

Successfully merging this pull request may close these issues.

2 participants