Monero: Code Style - Line Width Limit

[ilprincipe33]

Please consider formatting all sources by placing a strict column limit to (say) 80 columns to improve readability and maintainability.

Some source files are extremely difficult to read to the point where I had to use Clang Format before I could actually start reading the code...

Here is an example of a function that that is currently displayed as a single statement spanning 176 columns:

bool construct_tx_with_tx_key(const account_keys& sender_account_keys, const std::unordered_map<crypto::public_key, subaddress_index>& subaddresses, std::vector<tx_source_entry>& sources, const std::vector<tx_destination_entry>& destinations, const boost::optional<cryptonote::account_public_address>& change_addr, std::vector<uint8_t> extra, transaction& tx, uint64_t unlock_time, const crypto::secret_key &tx_key, const std::vector<crypto::secret_key> &additional_tx_keys, bool rct, bool bulletproof, rct::multisig_out *msout)

Source

Here is an example of aforementioned function styled with clang-format using Mozilla code style conventions:

bool
construct_tx_with_tx_key(
  const account_keys& sender_account_keys,
  const std::unordered_map<crypto::public_key, subaddress_index>& subaddresses,
  std::vector<tx_source_entry>& sources,
  const std::vector<tx_destination_entry>& destinations,
  const boost::optional<cryptonote::account_public_address>& change_addr,
  std::vector<uint8_t> extra, transaction& tx, uint64_t unlock_time,
  const crypto::secret_key& tx_key,
  const std::vector<crypto::secret_key>& additional_tx_keys, bool rct,
  bool bulletproof, rct::multisig_out* msout)

Hope this helps. GLHF.

[ilprincipe33]

Another thing: in C++ source files function definitions are being preceded by a comment line composed of hyphens that look like this:

//--------------------------------------------------------------------------------------
std::whatever_t core::my_method() const 
{
/* do whatever ... */
}

Presumably, these markers are being interpreted to facilitate code-folding specific to some editor(s). Whatever the case, this is just more cruft that makes code harder to read and should be removed.

[jasjuang]

+1 for clang-format, strict 80 column limit is needed

[moneromooo-monero]

NAK, no reformatting. If you're adding new code, you're free to keep to 80 columns, though.

[moneromooo-monero]

+invalid

[jasjuang]

@moneromooo-monero That's unfortunate. I was thinking about contributing to monero on my free time (I am sure @ilprincipe33 is too, that's why we are here), but the current no-coding-style is making the friction to contribute unnecessarily high.

[fluffypony]

@jasjuang then create an issue about establishing a coding style, and submit a PR after discussion in that issue. @moneromooo-monero is only saying that refactor-as-you-go is acceptable, but necessary refactoring is not.

[stoffu]

As someone who's been working on Monero for a little over a year, I'd say the codebase may look a bit overwhelming and intimidating at first sight, but it'll become quite manageable as you get used to it. Monero developers should have at least this level of patience and perseverance, IMO.