search

hasherezade / libpeconv

A library to load, manipulate, dump PE files. See also: https://github.com/hasherezade/libpeconv_tpl
https://hasherezade.github.io/libpeconv
BSD 2-Clause "Simplified" License
1.07k stars 176 forks source link

Code refactor and fixed possible bugs #40

Open GermanAizek opened 2 years ago

GermanAizek commented 2 years ago

@hasherezade, check it.

hasherezade commented 2 years ago

Sorry, but the majority of those changes give no real benefit for the project. They are just a different notation of the same, and a matter of coding style/taste, rather than giving any concrete gains. Yeah, there are various ways to define a default constructor, and there are various ways to check if the string is empty. You may have your preferred one, but it doesn't mean that the alternatives are wrong, and something to be fixed.

Maybe some strings should be defined as constant, but it is a low priority.

The only things that I could consider to be fixed is the typo in moduleSize:

size_t moduelSize;

and here:

if (name.length() == 0 || name.length() == 0) {

Still those are small things, and usually I don't consider them something worth for a merge, you could just leave a notice and they would be fixed anytime. Not really a need for a pull request, but sometimes I accept typo fixes.