[Snark] C++ myfunc1

Stephen Paul Weber singpolyma at singpolyma.net
Fri Nov 9 03:45:14 UTC 2018


>using namespace std;

Oh C++.

>class PigLatin {
>public:
>void pig(string string);
>void myfunc1(string string);
>private:
>int test();
>};

We have a class, but only one. Already a smell.

>bool a = false;

Hey, a global flag with a nondescript name! What could go wrong?

>int main(int argv, char **argc)

Always good to stuff our `main` in the same file as a random class 
definition.  Also good to use the traditional names for the variables, but 
backwards.

>       if( argv == 1 ){
>         return 0;

If we have no arguments (because argv == 1 is a totally clear way to check 
that...) we just exit success.  Not bad code, but certainly bad behaviour.

>       };

lol

>       a = true;

Of course it is.

>       PigLatin piglatin;
>       piglatin.myfunc1(argc[1]);

Whatever is in myfunc1 could definitely have been here.

>       cout << "\n"; }

No return at the end of main.

>void PigLatin::pig(string s_word)
>{
>	int a = 0;

Let's go ahead and shadow our flag with something else that looks bool-ish 
but isn't.

>	if( s_word[0] == 'a' || s_word[0] == 'e' || s_word[0] == 'i' || s_word[0] == 'o' || s_word[0] =='u' ||
>	(s_word.size() > 1 && ((s_word[0] == 'x' &&
>	s_word[1] == 'r') || (s_word[0] == 'y' && s_word[1] == 't'))))

This might actually be the easiest way to write this in C++, as much as that 
hurts me.  Doesn't handle case.

>	{
>	    string mystring;
>	    mystring = s_word + "ay";
>	    cout << mystring;
>	    return;

So close here.  Going to STDOUT right away to avoid an allocation... except, 
then making an allocation right here anyway.

>       for (int i = 0; a < s_word.size(); a++) {
>       	if (s_word[a] == 'a' || s_word[a] == 'e' || s_word[a] == 'i' || s_word[a] == 'o' || s_word[a] == 'u' || (s_word[a] == 'y' && 0 < a)) {
>       		a = s_word.size();

0 < a ? Where do we set 0 to be < 0?  Oh, maybe it could be == 0?  In fact, 
  we initialize it to 0.  But if we ever set it, it's to the size (length?  
are those the same?) of the word.  So only count 'y' if we've set a ever in 
a previous loop iteration.  Except that if a == s_word.size() then we break.  
Wait, so this is just an awkward way to break the loop?  Oh nice.  Oh ane we 
also a++ each iteration... so what is i for?  I don't think we use i, it's 
just there because for loops...

Oh, so 0 < a just means "is this the first iteration".

>       	if(s_word.size() > 1+a && s_word[a]=='q' && s_word[a+1] == 'u')
>       	{
>       		if (s_word[a] == 'q' && s_word[a+1] == 'u')

lol

>       };

lol

>	string mystring;
>	mystring = s_word.substr (str.length());
>	mystring = mystring  +str + "ay";
>	cout << mystring;

We just finished doing allocations to build str, but now we make *another* 
new string.  But then we just print it and throw everything away.

>	return ;
>}

Because main doesn't need a return, but this void function does.

>int PigLatin::test()
>{
>return 0;
>}

This function is never called.  And it's private.  And it does nothing.

>void PigLatin::myfunc1(string s) {
>	string usethis;

Good naming skills.  myfunc1. usethis.

>	for (int b = 0; b < s.size(); b++) {

Why b? Well, we already have a global `a`!

>		if(s[b] == ' ') {
>			if (b > 0) {
>	if (a == true) {

Ah, our old friend a == true

>		pig (usethis);
>		a = false;
>				}
>				else
>
>                               {
>                               	cout << " ";
>                               	pig(usethis);

I think the purpose of the global variable `a` is to emit a space or not 
before a non-first word.

>                               };

I love these.

>			myfunc1(s.substr(b + 1));

Oh, it's global because we recurse!  And we could never pass anything along, 
that would be crazy.  We're already inside a loop, but screw that, recursion 
time!

>			return;

And we don't actually want the loop any more after the recursion, so just 
return obviously.

>			// append s[b] to usethis
>			usethis += s[b];

Great comment, bro.

>		;

Ok, this one is actual gold.

>	};

Can't not comment.

>	if (usethis.length( ) > 0) { if (a!= 1) {

Wait. `a != 1` ?  a is our global bool.  So... `!a`?

>		};
>	}; }

Two more.

This one is nice, but my gut says some of the others this month were worse.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://snark.badcode.rocks/pipermail/snark/attachments/20181108/376fd2d2/attachment.sig>


More information about the Snark mailing list