Shift elements of an array

[alf.p.steinbach]

std::string is not an array.

Non-expert readers will more readily understand that you're expressing a subjective OPINION about TERMINOLOGY, instead of a fact about the technical, when you include e.g. "IMHO means" and refer to e.g. "terminology" or "the word".

It's an interesting view though, because it's unconventional, has no advantage that I can see, and certainly is very impractical.

So, could you perhaps elaborate on what you think the defining characteristics of an "array" are -- and where you picked up this view?

So your code have nothing common with the assignment.

I'm sorry, that's a fallacy.

Summing up, your posting consisted of (a) an impractical and unconventional subjective opinion about terminology, misleadingly expressed as an assertion about technical fact, and (2) a fallacy.


Cheers & hth.,

- Alf

 

[Vlad from Moscow]

It would be musch better if you would edd bla...bla..bla.. and learn what is array in C/C++. ALso you should read the original post where is is clearenough said what type of data orhanization have to be used in the assignment.
Moreover your example has totally different symantic because it does not shift a sequane but add a new element at the its beginning. So the size of the resulting sequence differs from the original sequence.


понедельник, 7 октÑÐ±Ñ€Ñ 2013 г., 3:07:33 UTC+4 пользователь (e-mail address removed) напиÑал:

 

[alf.p.steinbach]

First, please don't top post, and please do read up on that now.

Some people here, e.g. Victor, will simply ignore you when you do that.

Others, such as I, take it as an indication of the total newbie or troll.

I'm giving you the benefit of assuming that you're a total newbie, even though I do vaguely recall a "vlad" troll here some years ago, and even thoughfantasy statements, fallacies, and incoherence (exhibited below, first quote) are very strongly indicative of trolling (sorry for mentioning this, but necessary).

So,

---

It would be musch better if you would edd bla...bla..bla..

Sorry, that's incoherent, it reads like gobbledegook to me.

and learn what is array in C/C++.

I'm sorry, but there is no such language as "C/C++".

Notwithstanding the misleading title of a Microsoft book.

In particular, much modern C code is not valid as C++ code, some C code does compile as C++ but means something different in C++, and some best practices (e.g. whether to cast the result of malloc, if one uses malloc) are diametrically opposite, or differ, between the languages.

It's therefore best to not start out by conflating the languages.

Especially for a newbie.

ALso you should read the original post where is is clear enough said what
type of data orhanization have to be used in the assignment.

It's true that the type of data organization used by the OP is clear, namely an array.

There is no conflict between that and my simple suggestion of using std::string and its concatenation operation.

The answer may not, however, apply to all possible choices of item data. But the OP did not ask for that. The OP asked for "any suggestions" and got one, which happens to be much simpler than the answers so far, including yours.

Since I must assume that you are a newbie, as indicated by your top-posting, and as indicated by your ignorance of what arrays are, and more,

* the OP is most likely dealing with strings or data easily represented as strings,

* std::string provides constant time indexing, so it's a logical array, andit has a guaranteed contiguous buffer, so it's also an array by memory layout criterions (however misguided it would be to apply such criterions here), and

* in the cases where the provided solution fits it's very simple and clear and does the job, contrary to your opinion that it's irrelevant.

Moreover your example has totally different symantic because it does not
shift a sequane but add a new element at the its beginning.

Shifting to the right always adds a new element to the beginning.

If you don't introduce a new element then you're moving around the rightmost element, in which case it's called "rotating" rather than "shifting".

So the size of the resulting sequence differs from the original sequence.

Yes, the OP neglected to state anything about the size, so I did not add code to trim down the size.

If such code were added (it's trivial and very short, E.G. a call to .resize()) then one could equally well argue that it should not be there, depending on what one thinks is most likely the unstated requirement. For example,in arbitrary precision arithmetic, "bignum" arithmetic, shifting (to the left) does extend the sequence. The OP has not clarified what he/she wants.

I do not now and have never viewed comp.lang.c++ as a question-and-answer forum: while comp.lang.c++ does not have a charter, it's traditionally a discussion and learning forum, which is a very different kind of beast than Q&A. Newbie Q&A is best done in alt.comp.lang.learn.c-c++ or other groups dedicated to that sort of thing. Which is not to say that this group is not inclusive -- it is! -- but what kinds of answers can be considered appropriate or inappropriate or incomplete, is very different.

Anyway, in the absence of a specification it's best to not needlessly assume, and not needlessly add complicating details -- the OP can do that.


Cheers, and hope this helps! ;

- Alf

 

[Vlad from Moscow]

It seems you are indeed a troll. As I said your code !). has nothing commonwith the original assignment 2) has a totally different semantic. So you only confuse others and flood.

And it is your who should obviously be ignored.

понедельник, 7 октÑÐ±Ñ€Ñ 2013 г., 4:48:42 UTC+4 пользователь (e-mail address removed) напиÑал:

 

[Rosario1903]

On Sun, 06 Oct 2013 18:59:06 +0200, Rosario1903 wrote:
the last version for this exercise.

#include <iostream>
#include <cstdlib>

using namespace std;
#define u32 unsigned
#define u8 unsigned char

class myarray{
public:
~myarray(){free(arr);}
myarray(){arr=(u8*) malloc(15);
if(arr==0) exit(-1);
s=15;
}

myarray(u32 c)
{if(c>0xFFFF) exit(-1);
arr=(u8*)malloc(c);
if(arr==0) exit(-1);
s=c;
}

myarray(char* c)
{u32 v, i;
if(c==0)
{v=1;}
else {v=strlen(c)+1;}
arr=(u8*)malloc(v);
if(arr==0) exit(-1);
if(v==1) *arr=0;
else {for(i=0; i<v ; ++i)
{arr=c;
if(c==0) break;
}
for(; i<v;++i)
arr=0;
}
s=v;
}

friend ostream& operator<<(ostream& os, myarray& ma)
{u32 i;
if(ma.s!=0)
{for(i=0; i<ma.s-1; ++i)
os<<ma.arr;
return os<<ma.arr;
}
else return os;
}

u32 shiftd(myarray& ma, u32 pos)
{u32 i, j;
if(s<ma.s)
{free(arr);
arr=(u8*)malloc(ma.s);
if(arr==0) exit(-1);
s=ma.s;
}
if(pos>=ma.s)
{for(i=0;i<s; ++i) arr=0;}
else {/* 0 1 2 3 4 5 6 */
/* | shiftd 3*/
for(j=pos, i=0; j<ma.s; ++j,++i)
arr=ma.arr[j];
for(;i<s;++i)
arr=0;
}
return 1;
}

u32 s;
u8 *arr;
};


int main(void)
{myarray v("this and that "), ma;
u32 i;

cout<<"at start v=["<<v <<"]\n";
ma.shiftd(v, 3);
cout<<"at end ma=["<<ma<<"]\n";

for(i=0;i<12;++i)
{ma.shiftd(ma, 1);
cout<<"at end ma=["<<ma<<"]\n";
}

return 0;
}

---------------
at start v=[this and that ]
at end ma=[s and that ]
at end ma=[ and that ]
at end ma=[and that ]
at end ma=[nd that ]
at end ma=[d that ]
at end ma=[ that ]
at end ma=[that ]
at end ma=[hat ]
at end ma=[at ]
at end ma=[t ]
at end ma=[ ]
at end ma=[ ]
at end ma=[ ]

 

[alf.p.steinbach]

It seems you are indeed a troll. As I said your code !). has nothing common with the original assignment 2) has a totally different semantic. So you only confuse others and flood.

I'm sorry, that's quite lunatic..., except that

And it is your who should obviously be ignored.

.... you guessed that I would plink you. Good thinking.

Plink.


- Alf

 

[Rosario1903]

see if i know what he think above it is wrong

1)"using namespace std;" should be not used
[so i woudl use always std::etc]
2) #define u32 wrong for he
3) #define u8 wrong for he
[so i wuold write "unsigned int" all the code lenght; that for
write just one it take one day...]
4) using "public:" as class data [there would be private for him]
5) the using of malloc() for memory usage
6) the use of exit instead of use exception

but i'm not agree expecially point 5... using malloc() is always
easier [to debug too] than use new().

 

[alf.p.steinbach]

On Sun, 06 Oct 2013 18:59:06 +0200, Rosario1903 wrote:

the last version for this exercise.

Usually when people post code in comp.lang.c++, at least as of old they have expected a review. Else why post it? So, review.

First, I illustrate the main point, that the code is needlessly complicatedand verbose. The complexity also means that the code's brittle (will easily break in maintainance). Here's shorter and simpler code that produces essentially the same output as your program ("essentially": the lead text has been clarified):

#include <iostream>
#include <string>
using namespace std;

void shift_left( string& s, int n = 1 )
{ s = s.erase( 0, n ) + string( n, ' ' ); }

auto main() -> int
{
string s = "this and that";
cout << "at start:      '" << s << "'\n";

shift_left( s, 3 );
cout << "after shift 3: '" << s << "'\n";

for( int i = 1; i <= 12; ++i )
{
shift_left( s );
cout << "after shift 1: '" << s << "'\n";
}
}

#include <iostream>
#include <cstdlib>

using namespace std;

#define u32 unsigned
#define u8 unsigned char

Macros do not respect scopes, so they can easily lead to unintended text substitutions.

Therefore, to the degree possible, avoid macros.

And where you do use macros, adopt the (now) common convention of ALL UPPERCASE names for macros, which reduces the probability of unintended text substitution.

The u32 definition does not necessarily define a 32-bit type name.

class myarray{

public:

~myarray(){free(arr);}

Instead of C level malloc and free, use C++ new and delete.

And instead of C++ new and delete, preferentially use standard library containers such as std::vector, which deal automatically with deallocation.

One reason is that otherwise you will likely run afoul of the "rule of three" (or in C++11 sometimes now referred to as the "rule of five").

Essentially that means getting Undefined Behavior.

myarray(){arr=(u8*) malloc(15);

if(arr==0) exit(-1);

s=15;

}

Here with a std::string or std::vector you could just say

myarray(): arr( 15, ' ' ) {}

However the effect is /slightly/ different.

Your code produces an array of 15 items each of INDETERMINATE VALUE, while the suggested replacement produces an array of 15 items each with the spacecharacter code (which in ASCII is 32).

myarray(u32 c)

Since this generalizes the default constructor, you could code both up as the same constructor with a defaulted argument, like this:

myarray( u32 c = 15 )

However, in order to avoid problems with implicit promotions, which can cause unintended wrap-around with unsigned types, preferentially use a signed type, and also, to enhance readability preferentially use descriptive names:

myarray( int size = 15 )

{if(c>0xFFFF) exit(-1);

arr=(u8*)malloc(c);

As before, better use C++ new and delete, and wrt. those operators, preferentially instead use standard library containers.

if(arr==0) exit(-1);

As before, use "assert".

s=c;
}

Regarding the member initializations, in some cases one HAS TO use memory initializers, the ":" notation. And that's generally most efficient also. Soas a general rule, prefer ":" to assignments in a constructor body.

myarray(char* c)

Here the "char*" prevents the code from compiling with a conforming C++ compiler.

That's because the C implicit conversion from literal to non-const char* was deprecated in C++98 and REMOVED in C++11 (the current standard).

AFAIK most compilers still support that conversion, but they will probably not continue to support it for long.

So, use "char const*".

{u32 v, i;
if(c==0)
{v=1;}
else {v=strlen(c)+1;}
arr=(u8*)malloc(v);
if(arr==0) exit(-1);
if(v==1) *arr=0;
else {for(i=0; i<v ; ++i)
{arr=c;
if(c==0) break;
}
for(; i<v;++i)
arr=0;
}
s=v;
}

Generally the specific indentation rules don't matter as long as they're applied consistently.

However, the above indentation rules, which result in different indent sizes, is not very clear.

To some degree it defeats the purpose of indentation.

friend ostream& operator<<(ostream& os, myarray& ma)

The lack of "const" prevents passing a "const" myarray to <<.

Do like this:

friend ostream& operator<<( ostream&, myarray const& ma )

{u32 i;
if(ma.s!=0)

Better establish a guaranteed s != 0 in every constructor.

And don't change that property in any member function.

That's then called a CLASS INVARIANT, and can then simply be assumed everywhere without further checking.

{for(i=0; i<ma.s-1; ++i)
os<<ma.arr;
return os<<ma.arr;
}

else return os;
}


u32 shiftd(myarray& ma, u32 pos)

Ken Thompson (I think it was) was once asked what he'd do different if he were to design Unix again.

Answer: "I'd spell creat with an E".

Just avoid silly shortenings like "shiftd". Spell it out, like "shifted".

There is another possible improvement here, because a member function that updates this object's value with a copy of a modification of another object's value, is quite unusual and counter-intuitive.

Instead just provide a pure modifier member function, or a pure value producer member function.

{u32 i, j;
if(s<ma.s)
{free(arr);
arr=(u8*)malloc(ma.s);
if(arr==0) exit(-1);
s=ma.s;

}

if(pos>=ma.s)
{for(i=0;i<s; ++i) arr=0;}
else {/* 0 1 2 3 4 5 6 */
/* | shiftd 3*/
for(j=pos, i=0; j<ma.s; ++j,++i)
arr=ma.arr[j];
for(;i<s;++i)
arr=0;
}

return 1;

Always returning the same numeric value indicates that a numeric function result is not really meaningful.

Perhaps this member function should have been declared "void".

}

u32 s;
u8 *arr;

Generally, when a class has at least one constructor, then it's a good ideato make data members "private" or at least "protected", so as to avoid other code inadvertently changing those data members directly and e.g. invalidating the class invariant.

};

int main(void)

The "void" here is good practice in C, where it specifies that this function has no arguments, as opposed to accepting any number of arguments.

In C++ it's accepted just for C compatibility, but has no purpose: in C++ the signature "int main()" already says that there are no arguments.

So, it's a C-ism, which is best avoided -- because the two languages are fundamentally different, and should best not be conflated with each other.

{myarray v("this and that "), ma;

u32 i;

In C++ better declare variables as near their first use as possible.

In this case, write e.g. "for( u32 i = 0, ...".

However, as mentioned, it's even better to write just "for( int i = 0, ....".

cout<<"at start v=["<<v <<"]\n";
ma.shiftd(v, 3);
cout<<"at end ma=["<<ma<<"]\n";

for(i=0;i<12;++i)
{ma.shiftd(ma, 1);
cout<<"at end ma=["<<ma<<"]\n";
}

return 0;

In both C++ and C it's now unnecessary to return 0 from main, as that's thedefault.

It has always been the default in C++.

}

As a general comment, since myarray does not take charge of copying -- itdoes not declare any copy constructor or assignment operator -- any use of it that inadvertently invokes copying, will in general cause the same memory to be deallocated twice (via the free call in the destructor), invoking Undefined Behavior, such as e.g. a crash... :-(

In C++03 this is known as the "rule of three":

namely, if you need a destructor, a copy constructor or an assignment operator, then you most probably need ALL THREE.

---------------

at start v=[this and that ]
at end ma=[s and that ]
at end ma=[ and that ]
at end ma=[and that ]
at end ma=[nd that ]
at end ma=[d that ]
at end ma=[ that ]
at end ma=[that ]
at end ma=[hat ]
at end ma=[at ]
at end ma=[t ]
at end ma=[ ]
at end ma=[ ]
at end ma=[ ]

Again, note that the much shorter and simpler code shown at the start, produces essentially the same output.

Simpler and shorter = safer.


Cheers & hth.,

- Alf

 

[Rosario1903]

On Sun, 06 Oct 2013 18:59:06 +0200, Rosario1903 wrote:

the last version for this exercise.

Usually when people post code in comp.lang.c++, at least as of old they
have expected a review. Else why post it? So, review.

First, I illustrate the main point, that the code is needlessly
complicated and verbose. The complexity also means that the code's
brittle (will easily break in maintainance). Here's shorter and
simpler code that produces essentially the same output as your
program ("essentially": the lead text has been clarified):

#include <iostream>
#include <string>
using namespace std;

void shift_left( string& s, int n = 1 )
{ s = s.erase( 0, n ) + string( n, ' ' ); }[/QUOTE]

it is required from OP the result is in one other array
result!=argument
than it is required the array is 15 elements
[QUOTE]
As a general comment, since myarray does not take charge
of copying  --  it does not declare any copy constructor
or assignment operator  --  any use of it that
inadvertently invokes copying, will in general cause
the same memory to be deallocated twice (via the free
call in the destructor), invoking Undefined Behavior,
such as e.g. a crash... :-([/QUOTE]

if the compiler use constructor i wrote, and distructor i wrote there
are no problem
can you modify the code i wrote so i can see that? [a seg fault]
[QUOTE]
In C++03 this is known as the "rule of three":

namely, if you need a destructor, a copy constructor or an assignment
operator, then you most probably need ALL THREE. 
possibly
[QUOTE]
---------------

at start v=[this and that  ]
at end  ma=[s and that     ]
at end  ma=[ and that      ]
at end  ma=[and that       ]
at end  ma=[nd that        ]
at end  ma=[d that         ]
at end  ma=[ that          ]
at end  ma=[that           ]
at end  ma=[hat            ]
at end  ma=[at             ]
at end  ma=[t              ]
at end  ma=[               ]
at end  ma=[               ]
at end  ma=[               ][/QUOTE]

Again, note that the much shorter and simpler code shown at the start, produces essentially the same output.

Simpler and shorter = safer.


Cheers & hth.,

- Alf[/QUOTE]

you have confidence in call functions, i instead like loops...

 

[Rosario1903]

As a general comment, since myarray does not take charge
of copying -- it does not declare any copy constructor
or assignment operator -- any use of it that
inadvertently invokes copying, will in general cause
the same memory to be deallocated twice (via the free
call in the destructor), invoking Undefined Behavior,
such as e.g. a crash... :-(

if the compiler use constructor i wrote, and distructor i wrote there
are no problem
can you modify the code i wrote so i can see that? [a seg fault]

yes i find it... is is enougt add the function
myarray f(void)
{myarray a("1 2 3 4");
return a;
}

and put in some place in main
mystring ma;
ma=f()
cout <<"ma="<<ma<<"\n";

for see wrong ouput and seg fault...
but i not use that code in the example...
not use "operator="

In C++03 this is known as the "rule of three":

namely, if you need a destructor, a copy constructor or an assignment
operator, then you most probably need ALL THREE.
possibly

---------------

at start v=[this and that ]
at end ma=[s and that ]
at end ma=[ and that ]
at end ma=[and that ]
at end ma=[nd that ]
at end ma=[d that ]
at end ma=[ that ]
at end ma=[that ]
at end ma=[hat ]
at end ma=[at ]
at end ma=[t ]
at end ma=[ ]
at end ma=[ ]
at end ma=[ ]

Again, note that the much shorter and simpler code shown at the start, produces essentially the same output.

Simpler and shorter = safer.


Cheers & hth.,

- Alf

you have confidence in call functions, i instead like loops...

 

[Öö Tiib]

What exactly makes it not an array?

* "Array" in computer science is collection of elements where elements
can be indexed with numeric indexes. std::string is array in computer
science.

* In C++ we usually call raw array as "array". Other arrays we usually
call by specific names (like "vector", "valarray" or "string literal").
std::string is rarely referred as "array" in C++.

* OP is likely novice student so "correct" answer for her is whatever
her teacher expects. If Vlad from Moscow is her teacher then std::string
is not an array, otherwise it might be.

 

[Vlad from Moscow]

array is a derived type. It is not user-defined type. Arrays have no methods and they can not increase or decrease in size. So for example the semantic of "deleting" or "shifting" elements in an array is different than in a user-defined type such as for example std::vector. The size of an array haveto be known in compilation time.

 

[Vlad from Moscow]

You are wrong. You omit the important characteristic of arrays that they have fixed sizes. They are unable to add new elements orto delete existent elements. It is this their characteristic that is the reason that other containers as for example std::vector are required.

понедельник, 7 октÑÐ±Ñ€Ñ 2013 г., 21:31:09 UTC+4 пользователь Öö Tiib напиÑал:

 

[Vlad from Moscow]

And where did you see that his (or her) teacher spoke about std:;string?!!!I think that he is competent enough in contrast to this troll (e-mail address removed) and is capable to call things with their names.


- показать цитируемый текÑÑ‚ -

понедельник, 7 октÑÐ±Ñ€Ñ 2013 г., 21:31:09 UTC+4 пользователь Öö Tiib напиÑал:

 

[Vlad from Moscow]

Your code is ignorant.

Remember if you use containers you should use types defined in them. So at least the function should be declared as

void shift_left( std::string& s, std::string::size_type n = 1 );

Moreover in this statement

s = s.erase( 0, n ) + string( n, ' ' );

you allocate a new string. It is simply a stupidy.

And the return type void in this case is the worst return type.


понедельник, 7 октÑÐ±Ñ€Ñ 2013 г., 11:07:52 UTC+4 пользователь (e-mail address removed) напиÑал:

the last version for this exercise.

Usually when people post code in comp.lang.c++, at least as of old they have expected a review. Else why post it? So, review.



First, I illustrate the main point, that the code is needlessly complicated and verbose. The complexity also means that the code's brittle (will easily break in maintainance). Here's shorter and simpler code that produces essentially the same output as your program ("essentially": the lead text has been clarified):

#include <iostream>

#include <string>

using namespace std;



void shift_left( string& s, int n = 1 )

{ s = s.erase( 0, n ) + string( n, ' ' ); }



auto main() -> int

{

string s = "this and that";

cout << "at start:      '" << s << "'\n";



shift_left( s, 3 );

cout << "after shift 3: '" << s << "'\n";



for( int i = 1; i <= 12; ++i )

{

shift_left( s );

cout << "after shift 1: '" << s << "'\n";

}

}

#include <iostream>

#include <cstdlib>

using namespace std;

#define u32 unsigned

#define u8 unsigned char

Macros do not respect scopes, so they can easily lead to unintended text substitutions.



Therefore, to the degree possible, avoid macros.



And where you do use macros, adopt the (now) common convention of ALL UPPERCASE names for macros, which reduces the probability of unintended text substitution.



The u32 definition does not necessarily define a 32-bit type name.



If you really want a 32-bit thing, use the <stdint.h> header.

class myarray{



~myarray(){free(arr);}

Instead of C level malloc and free, use C++ new and delete.



And instead of C++ new and delete, preferentially use standard library containers such as std::vector, which deal automatically with deallocation.



One reason is that otherwise you will likely run afoul of the "rule of three" (or in C++11 sometimes now referred to as the "rule of five").



Essentially that means getting Undefined Behavior.

myarray(){arr=(u8*) malloc(15);

if(arr==0) exit(-1);



}

Here with a std::string or std::vector you could just say



myarray(): arr( 15, ' ' ) {}



However the effect is /slightly/ different.



Your code produces an array of 15 items each of INDETERMINATE VALUE, while the suggested replacement produces an array of 15 items each with the space character code (which in ASCII is 32).

myarray(u32 c)

Since this generalizes the default constructor, you could code both up asthe same constructor with a defaulted argument, like this:



myarray( u32 c = 15 )



However, in order to avoid problems with implicit promotions, which can cause unintended wrap-around with unsigned types, preferentially use a signed type, and also, to enhance readability preferentially use descriptive names:



myarray( int size = 15 )

{if(c>0xFFFF) exit(-1);

Here better use an "assert", from <assert.h>.

arr=(u8*)malloc(c);

As before, better use C++ new and delete, and wrt. those operators, preferentially instead use standard library containers.

if(arr==0) exit(-1);

As before, use "assert".

}

Regarding the member initializations, in some cases one HAS TO use memoryinitializers, the ":" notation. And that's generally most efficient also. So as a general rule, prefer ":" to assignments in a constructor body.

myarray(char* c)

Here the "char*" prevents the code from compiling with a conforming C++ compiler.



That's because the C implicit conversion from literal to non-const char* was deprecated in C++98 and REMOVED in C++11 (the current standard).



AFAIK most compilers still support that conversion, but they will probably not continue to support it for long.



So, use "char const*".

{u32 v, i;


else {v=strlen(c)+1;}

if(arr==0) exit(-1);

if(v==1) *arr=0;

else {for(i=0; i<v ; ++i)

{arr=c;

if(c==0) break;

}

for(; i<v;++i)

arr=0;

}

s=v;

}

Generally the specific indentation rules don't matter as long as they're applied consistently.



However, the above indentation rules, which result in different indent sizes, is not very clear.



To some degree it defeats the purpose of indentation.

friend ostream& operator<<(ostream& os, myarray& ma)

The lack of "const" prevents passing a "const" myarray to <<.



Do like this:



friend ostream& operator<<( ostream&, myarray const& ma )

{u32 i;

if(ma.s!=0)

Better establish a guaranteed s != 0 in every constructor.



And don't change that property in any member function.



That's then called a CLASS INVARIANT, and can then simply be assumed everywhere without further checking.

{for(i=0; i<ma.s-1; ++i)

os<<ma.arr;

return os<<ma.arr;

}

else return os;

}

u32 shiftd(myarray& ma, u32 pos)

Ken Thompson (I think it was) was once asked what he'd do different if hewere to design Unix again.



Answer: "I'd spell creat with an E".



Just avoid silly shortenings like "shiftd". Spell it out, like "shifted".



There is another possible improvement here, because a member function that updates this object's value with a copy of a modification of another object's value, is quite unusual and counter-intuitive.



Instead just provide a pure modifier member function, or a pure value producer member function.

{u32 i, j;

if(s<ma.s)

{free(arr);

arr=(u8*)malloc(ma.s);

if(arr==0) exit(-1);

s=ma.s;

}

if(pos>=ma.s)

{for(i=0;i<s; ++i) arr=0;}

else {/* 0 1 2 3 4 5 6 */

/* | shiftd 3*/

for(j=pos, i=0; j<ma.s; ++j,++i)

arr=ma.arr[j];

for(;i<s;++i)

arr=0;

}

return 1;

Always returning the same numeric value indicates that a numeric functionresult is not really meaningful.



Perhaps this member function should have been declared "void".

}

u32 s;

u8 *arr;

Generally, when a class has at least one constructor, then it's a good idea to make data members "private" or at least "protected", so as to avoid other code inadvertently changing those data members directly and e.g. invalidating the class invariant.

};

int main(void)

The "void" here is good practice in C, where it specifies that this function has no arguments, as opposed to accepting any number of arguments.



In C++ it's accepted just for C compatibility, but has no purpose: in C++the signature "int main()" already says that there are no arguments.



So, it's a C-ism, which is best avoided -- because the two languages are fundamentally different, and should best not be conflated with each other.

{myarray v("this and that "), ma;

u32 i;

In C++ better declare variables as near their first use as possible.



In this case, write e.g. "for( u32 i = 0, ...".



However, as mentioned, it's even better to write just "for( int i = 0, ....".

cout<<"at start v=["<<v <<"]\n";

ma.shiftd(v, 3);

cout<<"at end ma=["<<ma<<"]\n";

for(i=0;i<12;++i)

{ma.shiftd(ma, 1);

cout<<"at end ma=["<<ma<<"]\n";

}

return 0;

In both C++ and C it's now unnecessary to return 0 from main, as that's the default.



It has always been the default in C++.



However, many programmers still prefer to indicate the "main" return value explicitly. For that purpose it can be a good idea to use the symbolic values EXIT_SUCCESS and EXIT_FAILURE from <stdlib.h>. In this case, EXIT_SUCCESS, which is what 0 means in this context.

}

As a general comment, since myarray does not take charge of copying -- it does not declare any copy constructor or assignment operator -- any use of it that inadvertently invokes copying, will in general cause the same memory to be deallocated twice (via the free call in the destructor), invoking Undefined Behavior, such as e.g. a crash... :-(



In C++03 this is known as the "rule of three":



namely, if you need a destructor, a copy constructor or an assignment operator, then you most probably need ALL THREE.

at end ma=[s and that ]

at end ma=[ and that ]

at end ma=[and that ]

at end ma=[nd that ]

at end ma=[d that ]

at end ma=[ that ]

at end ma=[that ]

at end ma=[hat ]

at end ma=[at ]

at end ma=[t ]

at end ma=[ ]

at end ma=[ ]

at end ma=[ ]

Again, note that the much shorter and simpler code shown at the start, produces essentially the same output.



Simpler and shorter = safer.





Cheers & hth.,



- Alf

 

[Ian Collins]

Your code is ignorant.

Please stop top-posting (it's ignorant behaviour) and and quoting all
the added crap that shite google interface adds.

 

[Rosario1903]

That's because the C implicit conversion from literal
to non-const char* was deprecated in C++98 and REMOVED
in C++11 (the current standard).

one other error
there is no need of this ungly word "const"

AFAIK most compilers still support that conversion,
but they will probably not continue to support it for long.

So, use "char const*".

#include <iostream>
#include <cstdlib>

using namespace std;

#define u32 unsigned
#define u8 unsigned char
#define F for
#define R return

class myarray{
public:
~myarray(){free(arr);}
myarray(){arr=(u8*) malloc(15);
if(arr==0) exit(-1);
s=15;
}

myarray(u32 c)
{if(c>0xFFFF) exit(-1);
arr=(u8*)malloc(c);
if(arr==0) exit(-1);
s=c;
}

myarray(char* c)
{u32 v, i;
if(c==0)
{v=1;}
else {v=strlen(c)+1;}
arr=(u8*)malloc(v);
if(arr==0) exit(-1);
if(v==1) *arr=0;
else {F(i=0; i<v ; ++i)
{arr=c;
if(c==0) break;
}
F(; i<v;++i)
arr=0;
}
s=v;
}

myarray& operator=(const myarray& r)
/* here i had to use the ugnly word "const" */
{u32 i;

if(s<r.s)
{free(arr);
arr=(u8*) malloc(r.s);
if(arr==0) exit(-1);
s=r.s;
}
F(i=0; i<r.s; ++i)
arr=r.arr;
R *this;
}

friend ostream& operator<<(ostream& os, myarray& ma)
{u32 i;
if(ma.s!=0)
{F(i=0; i<ma.s-1; ++i)
os<<ma.arr;
R os<<ma.arr;
}
else R os;
}

void shiftd(myarray& ma, u32 pos)
{u32 i, j;
if(s<ma.s)
{free(arr);
arr=(u8*)malloc(ma.s);
if(arr==0) exit(-1);
s=ma.s;
}
if(pos>=ma.s)
{F(i=0;i<s; ++i) arr=0;}
else {/* 0 1 2 3 4 5 6 */
/* | shiftd 3*/
F(j=pos, i=0; j<ma.s; ++j,++i)
arr=ma.arr[j];
F(;i<s;++i)
arr=0;
}
}


u32 s;
u8 *arr;
};

myarray *bi[50]={0};
u32 index=0;

void fill_bi(myarray* a)
{if(index==49) exit(-1);
bi[index]=a;
++index;
}

void free_bi(void)
{myarray *a;
u32 i;

if(index!=0)
{F(i=index-1; ; --i)
{a=bi;
free(a->arr);
free(a);
bi=0;
if(i==0) break;
}
}
index=0;
}

myarray& f(void)
{myarray a("1 2 3 4"), *b;

b=(myarray*)malloc(sizeof *b);
if(b==0) exit(-1);
b->arr=(u8*)malloc(15);
if(b->arr==0) exit(-1);
b->s=15;
fill_bi(b);

*b=a;
R *b;
}


int main(void)
{myarray v("this and that "), ma, b;
u32 i;

cout<<"at start v=["<<v <<"]\n";
ma.shiftd(v, 3);
cout<<"at end ma=["<<ma<<"]\n";

F(i=0;i<12;++i)
{ma.shiftd(ma, 1);
cout<<"at end ma=["<<ma<<"]\n";
}

b=v;
cout<<"b="<<b<<"\n";
b=f();
free_bi();

cout<<"b="<<b<<"\n";
R 0;
}

 

[Vlad from Moscow]

It is an example of how code shall not be written.

понедельник, 7 октÑÐ±Ñ€Ñ 2013 г., 11:07:52 UTC+4 пользователь (e-mail address removed) напиÑал:

the last version for this exercise.

Usually when people post code in comp.lang.c++, at least as of old they have expected a review. Else why post it? So, review.



First, I illustrate the main point, that the code is needlessly complicated and verbose. The complexity also means that the code's brittle (will easily break in maintainance). Here's shorter and simpler code that produces essentially the same output as your program ("essentially": the lead text has been clarified):

#include <iostream>

#include <string>

using namespace std;



void shift_left( string& s, int n = 1 )

{ s = s.erase( 0, n ) + string( n, ' ' ); }



auto main() -> int

{

string s = "this and that";

cout << "at start:      '" << s << "'\n";



shift_left( s, 3 );

cout << "after shift 3: '" << s << "'\n";



for( int i = 1; i <= 12; ++i )

{

shift_left( s );

cout << "after shift 1: '" << s << "'\n";

}

}

 

[Rosario1903]

it is required from OP the result is in one other array
result!=argument

than it is required the array is 15 elements

even if it not seems, all myarray object would have size==15 there...

 

[Vlad from Moscow]

It will be funny to count how many times the memory is deallocated and allocated anew and how many times constructors including the move assignment operator are called in this stupid statement.

{ s = s.erase( 0, n ) + string( n, ' ' ); }


понедельник, 7 октÑÐ±Ñ€Ñ 2013 г., 11:07:52 UTC+4 пользователь (e-mail address removed) напиÑал:

the last version for this exercise.

Usually when people post code in comp.lang.c++, at least as of old they have expected a review. Else why post it? So, review.



First, I illustrate the main point, that the code is needlessly complicated and verbose. The complexity also means that the code's brittle (will easily break in maintainance). Here's shorter and simpler code that produces essentially the same output as your program ("essentially": the lead text has been clarified):

#include <iostream>

#include <string>

using namespace std;



void shift_left( string& s, int n = 1 )

{ s = s.erase( 0, n ) + string( n, ' ' ); }
[/QUOTE]