Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    text
    copied!<p>First off I'd recommend to not use that vast list of globals. Use parameters in functions and return values from functions.</p> <p>Second <code>isdigit</code> does not work like that. It takes a parameter and return true or false. <a href="http://www.cplusplus.com/reference/clibrary/cctype/isdigit/" rel="nofollow">isdigit</a></p> <p>Further I'd use <a href="http://www.cplusplus.com/reference/clibrary/cstdio/getchar/" rel="nofollow">getchar</a> over scanf.</p> <p>Your <code>int check_dig()</code> function is a bit strange. You keep reading characters inside the function.</p> <p>I would perhaps do something like:</p> <pre><code>int valid_csv(char c) { if (isdigit(c)) return 1; /* etc. other checks */ if all fails return 0; } int main(void) { int c; /* read chars into c and call fun by something like */ valid_svc(c); return 0; } </code></pre> <hr> <p>Edit: As a rule of thumb. A function should do one thing, and one thing only, and do it well. The name of the function should reflect what it does.</p> <hr> <p>Edit.2:</p> <p>You do not need to "skip char". The way it goes, in your new code, is that you skip every other character.</p> <p>I.e.:</p> <p>File: 12,33,66,14</p> <p>In your code you'll get </p> <ul> <li>c = getchar => c == 1</li> <li>c is digit <ul> <li>getchar => you read 2 (and never validates it)</li> </ul></li> <li>c = getchar => c == ,</li> <li>c is comma <ul> <li>getchar => you read 3 (and never validates it)</li> </ul></li> <li>...</li> </ul> <p>Further; I know I wrote "a function should do one thing" - but but not that literal. Ie your new <code>check_digit</code> is redundant. Use <code>isdigit</code> directly. If you have floats in your csv you'll have to expand or use a different approach.</p> <p>To illustrate by example; easier then writing on here :)</p> <pre><code>#include &lt;ctype.h&gt; #include &lt;stdio.h&gt; int valid_csv_chr(int); int valid_csv(); /* guess naming could be better. */ int main(void) { if (valid_csv()) puts("1"); else puts("0"); return 0; /* Main should return 0 if there was no "crash" scenario etc. * You could also return i.e. 1 if the file is not validated as csv. * Do not think of 1 and 0 as boolean true / false here. */ } int valid_csv() { int c; while((c = getchar()) !=EOF) { if (!valid_csv_chr(c)) { return 0; } } return 1; } int valid_csv_chr(int c) { if (isdigit(c)) return 1; if (c == ',') return 1; if (c == '\n') return 1; /* add checks for space etc. */ return 0; } </code></pre> <hr> <p>Edit.3:</p> <p>Code structure is something one has to learn just as much as the language itself. It is learning by doing and writing. That one realize that re-structure of code is necessary happens from time to time - but the more one think before writing, make a simple structure at first and expand it, etc. it is more avoidable.</p> <p>Anyhow; practice, practice, practice. And keep these topics in mind at all time.</p> <p>Even if it can "look" simple, it is not. I think often books, tutorials, courses etc. have all to little focus on this topic. It is all about <code>for</code>, <code>if</code>, functions etc. and all to little on how to stitch it all together in a <strong><em>good</em></strong> way.</p> <p>There are several advantages of splitting up the code.</p> <ul> <li>It makes it much more readable.</li> <li>It makes it easier to maintain.</li> <li>Bugs and errors can often be fixed by fixing a small function rather then a huge one. <ul> <li>On some cases I have seen code of thousands of lines where one have some monster functions that is buggy. Fixing by tweaking it is close to impossible and a complete re-write is only option.</li> </ul></li> <li>It is easier to optimize a function that does one task, and is not to big.</li> <li>It is easier to expand to cover more scenarios when using smaller functions. <ul> <li>Say for example in your program. You could change it to "validate data file" covering csv, tab delimited, aligned, etc.</li> </ul></li> </ul> <p>The last point is actually a way I often think of it when writing; "How should I best implement this code so that, if, in the future, I'd like to expand it to cover more scenarios, it can be easily done."</p> <p>Myself I use <a href="http://lxr.linux.no/#linux+v3.3.1/Documentation/CodingStyle" rel="nofollow">this</a> as a <strong><em>base</em></strong> when writing in C combined with things I've learned in K &amp; R's ANSI C book ++. See for example what is written about <a href="http://lxr.linux.no/#linux+v3.3.1/Documentation/CodingStyle#L344" rel="nofollow">functions</a>.</p> <p>Also; being strict on coding style makes it much easier to read and maintain. I use in large part what is described in the document above. It is no <em>law</em>, but being conscious about it makes the coding life so much simpler.</p>
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload